PNG Library #817
PNG Library #817
Conversation
…hat its supposed to be tho
| s->windowReadOffs = ((word) s->read) - ((word) s->window); \ | ||
| s->windowWriteOffs = ((word) s->write) - ((word) s->window); \ | ||
| MemUnlock(s->windowHan); \ | ||
| } |
There was a problem hiding this comment.
Perhaps it makes sense to add some EC code here that e.g. sets the window ptr to NULL when it is unlocked, and asserts that it is null when locking (just so there is some checking that we don't have code paths that use them incorrectly)
There was a problem hiding this comment.
I have never written asserts and EC Checks, can you give an example please....?
| } | ||
|
|
||
| #define IF_GEOS_UNLOCK_SLIDING_WINDOW(s) { \ | ||
| s->windowEndOffs = ((word) s->window) + ((word) s->windowSize); \ |
There was a problem hiding this comment.
Shouldn't this be s->windowEndOffs = ((word) s->end) - ((word) s->window);?
Alternatively, something like PtrToOffset(s->end) should also work, as the offset of s->window should always be zero.
There was a problem hiding this comment.
I tried, and while I understand what you mean, your suggestion doesn't work - the original code however, does...
Library/Breadbox/zlib/INFLATE.c
Outdated
| @@ -159,134 +165,167 @@ int stream_size; | |||
| #define NEEDBYTE {if(z->avail_in==0)return r;r=f;} | |||
There was a problem hiding this comment.
Strictly speaking, this macro also contains a return statement that would need to call IF_GEOS_UNLOCK_SLIDING_WINDOW (even if it is just an error condition for corrupt data...). Perhaps it is easier to move the IF_GEOS_LOCK_SLIDING_WINDOW into the case statement, so the locking only happens for cases the really need to access the dictionary. This would require some more locks, but less unlocks, I think.
There was a problem hiding this comment.
Very good find. While you are probably right, I will for now just add IF_GEOS_UNLOCK_SLIDING_WINDOW to the macro... is that correct?
| IF_GEOS_LOCK_SLIDING_WINDOW(s); | ||
| inflate_blocks_reset(s, z, Z_NULL); | ||
| IF_GEOS_UNLOCK_SLIDING_WINDOW(s); | ||
| s->windowEndOffs = 0; |
There was a problem hiding this comment.
Probably not needed, the block will be immediately freed anyway...
There was a problem hiding this comment.
You are right! But "inflate_block_reset" sets
s->read = s->write = s->window;
That's why I did it. Will it be safe to set these pointers before deleting the window?
|
One more observation: I find that the image in the test case I put on http://www.mgroeber.de/person.html does not load, probably because import starts too early, while the file is still being streamed. By contrast, http://www.w3.org/Graphics/PNG/Alphatest.html loads fine (and imports progressively). |
| unfilterRow(state->currentRow, state->previousRow, state->bytesPerPixel, rowBytes); | ||
|
|
||
| // Move the pixel data in the row buffer to skip the filter byte | ||
| memmove(state->currentRow, state->currentRow + 1, rowBytes); |
There was a problem hiding this comment.
Could we save one copy by combining this move with the next memcpy, i.e. directly copy currentRow + 1 to previousRow?
There was a problem hiding this comment.
To be honest, we probably could, but I spent so many hours getting this right that I'm really afraid of touching this code ;-)
… to prevent things from staying locked in case of early return
After reverting some of the changes in the browser, this page loads for me with the image (even if it takes a while....) |
This PR adds a simple, mostly AI generated PNG (Import) Library to the PC/GEOS system, therefore partly resolving issue #184 . The most commonly used format specifications are supported. Creating this lib was only possible due to the fact that @rabe-soft ported the zlib library long ago.
Not supported are PNG "background"- and "transparency"-chunks as well as interlaced PNGs. Image width is currently limited to some ~1600 pixels. This may change with the upcoming protected mode support.
At the moment, the PNG support is limited to importing images in the "Graphics Viewer" app and the ImpGraph library (Browser).