On 10/24/2012 02:18 PM, Vincent Povirk wrote:
>> I found some code that reads a generic chunk. I see where the size
>> and type are read, where a correctly sized record is allocated and
>> where the data is read into the record.
>> That code is unused for reading and writing PNG images right now,
> sorry for the confusion. Libpng has its own code for reading and
> writing PNG chunks, and that is what is being used. Our chunk code is
> used for PNG metadata handlers (which will have to work independently
> from libpng so we can have a proper, pluggable system for metadata),
> but the architecture isn't there yet to use those handlers when
> reading a PNG image.
>>> I added code to read and check the CRC to my working copy of the git
>> tree. That threw off the block synchronization, so I think there is
>> code someplace else that either checks or skips the CRC, but I have
>> not found it after a few hours searching for it. An indication on
>> where to look and other advice would be very helpful at this time.
>> None of that code should be used yet in a file with more than one
> block, so I don't know how this could be causing problems. My plan was
> that it would be up to the caller to seek to the next block based on
> the returned data size.
>Hmm. With the version that reads the CRC, I get more error messages
and failures doing a 'make test' than the version without the addition.
>> Second: At least two of the methods GuildWars2 wants to use are stubbed.
>> To implement those methods, the frame section of the WIC object should
>> contain an array (or something with similar properties) of pointers to
>> chunks. From what I have read about WIC, other formats could use a
>> similar array. Some could even use an array of pointers to frames.
>> Before I go messing with that level of change, I think I should listen
>> to other peoples opinions of the subject.
>> Please keep in mind that Guild Wars 2 is probably not using WIC
> directly. You should find out which library it's actually using
> (probably d3dx9, but could also be gdiplus or oleaut32's IPicture
> interface) and take that into account. We can modify users of WIC to
> use it in efficient ways, and/or optimize WIC for the way we are using
> it. (MSDN recommends reading the entire image once either in one
> CopyPixels call or in rows from top to bottom, and optimizing for that
> case. We don't do that yet because we needed the general case first.)
>d3dx9. From comments from the developers, they were having trouble
with slow access to their data file. It's a huge thing, 30+GB. (Yes,
that is a G, not an M). I'd tried a patch on GetCount (which completely
misunderstood) and it went on to do something 'ReaderByIndex'. It put
more images on the screen much faster for some reason. I have very very
little information on the game's internal structure other than what
wine logs.
>> Third: On a very broad level, the whole OLE implementation seems to be
>> very messed up. In particular, the usual practice for doing
>> inheritance in C does not seem to be being used. That practice is to
>> have the elements common to the base and extending object be placed at
>> the beginning of the implementing data structures. While the member
>> names need not be the same in all instances, the function, type and
>> order of the common elements must be the same for economical
>> implementation.
>> I don't think a base class would've make anything easier.
>>> Is there any reason, other than inertia, that at least these two
>> fields should not be moved to the beginning of implementing objects.
>> This can be done one object at a time and would allow the changed
>> objects to use a common implementation of 'AddRef' at least, and
>> another common routine or macro that handles the critical part of
>> 'Release'.
>> We still couldn't do that because every interface vtable pointer would
> be at a different offset within the struct. We couldn't share them
> between encoders because different encoders have different data that
> they need to free. Anyway, it's not likely to have a measurable impact
> on performance.
>OK. I was thinking every object based on IUnknown would start with a
VTable pointer, a critical section lock and a reference count. I did
not expect to see any performance improvement, just the absence of
innumerable 'AddRef' and 'Release' variants. In fact I sort of expected
both of them to go away as virtual functions with a virtual destructor
in their place.
> Could you share some results of your profiling? What is it that led
> you to believe the PNG code is a bottleneck?
>Not allowed to profile as such. It was just the difference in speed
when the game thought it had 'PNGGetCount'