From: Francesco Montorsi on
Vadim Zeitlin ha scritto:
> FM> Here the problem is that, as documented, wxZipInputStream is not a
> FM> seekable stream. wxICOHandler, except for the initial SeekI(0), seems
> FM> to need seekable streams only in line 1323 of imagbmp.cpp, i.e. when
> FM> loading a specific icon from a multi-icon .ico file.
>
> Even this could be avoided as currently we have
>
> wxFileOffset iPos = stream.TellI();
> ...
> stream.SeekI(iPos + wxUINT32_SWAP_ON_BE(pCurrentEntry->dwImageOffset),
> wxFromStart);
>
> which could be rewritten as
>
> stream.SeekI(wxUINT32_SWAP_ON_BE(pCurrentEntry->dwImageOffset),
> wxFromCurrent);
>
> and seeking a positive amount from the current position could be
> implemented for any stream by just reading and discarding the given amount
> of data.
indeed; that's what was suggested in http://trac.wxwidgets.org/ticket/10373


> FM> The initial SeekI(0) is present in almost all wx*Handler classes.
>
> Yes, and IMO it's wrong to have it there. CanRead() really shouldn't
> modify the stream position in the first place and so it should rewind the
> stream back if it changed it. As for LoadFile(), it shouldn't do anything
> at all with the stream position.
I agree

> FM> I think it's necessary because loading an image implies that
> FM> wxICOHandler::DoCanRead(), which moves the "current position" in the stream, is
> FM> called before wxICOHandler::DoLoadFile.
>
> This is only necessary if you accept that DoCanRead() changes the current
> position which doesn't make much sense: why should testing something change
> the object being tested?
it shouldn't in fact...

> FM> I see two possible solutions:
> FM> 1) reset the current position in the stream in the wxICOHandler::DoCanRead()
> FM> function, instead of doing it in DoLoadFile (so that if you call directly
> FM> DoLoadFile, it may work also on non-seekable streams).
>
> This is the one which I prefer, by far.
I've implemented it in r60852, together with other related changes (e.g. the
same problem is present also for GetImageCount()).


Francesco

From: Francesco Montorsi on
Hi,

Matthew Fanto ha scritto:
> On Thu, May 28, 2009 at 4:12 PM, Francesco Montorsi
> <f18m_cpp217828(a)yahoo.it <mailto:f18m_cpp217828(a)yahoo.it>> wrote:
>
>
> Could you test this change and see if your icon gets loaded?
> If the code works then it needs to be copied also in
> wxBufferedInputStream (see the initial comment in wxInputStream::SeekI).
>
> Yes, the patch did correct the problem and it now works. The icon is
> loaded, and no errors are given. Thank you very much.
good, thanks for the feedback.

I've now fixed the problem in a more extensive way in wx trunk. If you can test
the changes and report any problem, it would be very nice. TIA!

Francesco