2014-07-22 1:29 GMT+02:00 <commits at source.squeak.org>:

> Nicolas Cellier uploaded a new version of Graphics to project The Trunk:
> http://source.squeak.org/trunk/Graphics-nice.296.mcz
>
> ==================== Summary ====================
>
> Name: Graphics-nice.296
> Author: nice
> Time: 22 July 2014, 1:28:48.356 am
> UUID: 3f6d869e-9929-4e08-b137-aae22dd3eee6
> Ancestors: Graphics-nice.295
>
> Don't try to accelerate GIF reading by copying the stream in memory.
> This should be the purpose of a decent file buffering policy, and if ever
> that was not the case, then we should fix the file buffering policy rather
> than trying to patch each and every client site.
>
> Especially when the patch is testing that the stream is in memory with a
> class identity to ReadWriteStream!!!
> Frankly, how do we know that it's not a RWBinaryOrTextStream ;)
>
> So why reading an image with a ReadWriteStream?
> Oh, sure, this is in ImageReadWriter, so we may need a ReadWriteStream?
> No, no, and no!
> This thing will either read or write but NEVER read and write.
> One simple role, one simple class, no UberPotentSwissKnifeStream with
> UberComplexStateToManage (readPosition + writePosition + readLimit +
> writeLimit + readBuffer + writeBuffer + readWriteBufferInteraction).
>
> What's coming next?
> Easy to guess, see what (ImageReadWriter>>on:) does frivously to our
> stream:
> reset??? Hey, ImageReadWriter, why do you take the responsibility to reset
> the stream? Did you ask the permission? Did you think of possible side
> effects?
>

the answer is summarized there:

readNativeResourceFrom: byteStream
    | img aStream |
    (byteStream isKindOf: FileStream) ifTrue:[
        "Ugly, but ImageReadWriter will send #reset which is implemented as
#reopen and we may not be able to do so."
        aStream := RWBinaryOrTextStream with: byteStream contents.
    ] ifFalse:[
        aStream := byteStream.
    ].

Ugly says it all.



> I'm quite sure those reset has something to do with following usage:
>     (ReadWriteStream with: 'foo') next -> nil
> but:
>     (ReadWriteStream with: 'foo') reset; next -> $f
> Pure Read/Write madness.
> There is more room for cleanup here, but only after I get rid of more
> R/WThing (TM), otherwise I could break some insane expectations.
>
> =============== Diff against Graphics-nice.295 ===============
>
> Item was changed:
>   ----- Method: AnimatedGIFReadWriter>>allImages (in category 'accessing')
> -----
>   allImages
>         | body colorTable |
> -       stream class == ReadWriteStream ifFalse: [
> -               stream binary.
> -               self on: (ReadWriteStream with: (stream
> contentsOfEntireFile))].
>         localColorTable := nil.
>         forms := OrderedCollection new.
>         delays := OrderedCollection new.
>         comments := OrderedCollection new.
>         self readHeader.
>         [(body := self readBody) == nil]
>                 whileFalse: [colorTable := localColorTable
>                                                 ifNil: [colorPalette].
>                         transparentIndex
>                                 ifNotNil: [transparentIndex + 1 >
> colorTable size
>                                                 ifTrue: [colorTable :=
> colorTable forceTo: transparentIndex + 1 paddingWith: Color white].
>                                         colorTable at: transparentIndex +
> 1 put: Color transparent].
>                         body colors: colorTable.
>                         forms add: body.
>                         delays add: delay].
>         ^ forms!
>
> Item was changed:
>   ----- Method: GIFReadWriter>>nextImage (in category 'accessing') -----
>   nextImage
> +       "Read in the next GIF image from the stream."
> -       "Read in the next GIF image from the stream. Read it all into
> - memory first for speed."
>
>         | f thisImageColorTable |
> -       stream class == ReadWriteStream ifFalse: [
> -               stream binary.
> -               self on: (ReadWriteStream with: (stream
> contentsOfEntireFile))].
>
>         localColorTable := nil.
>         self readHeader.
>         f := self readBody.
>         self close.
>         f == nil ifTrue: [^ self error: 'corrupt GIF file'].
>
>         thisImageColorTable := localColorTable ifNil: [colorPalette].
>         transparentIndex ifNotNil: [
>                 transparentIndex + 1 > thisImageColorTable size ifTrue: [
>                         thisImageColorTable := thisImageColorTable
>                                 forceTo: transparentIndex + 1
>                                 paddingWith: Color white
>                 ].
>                 thisImageColorTable at: transparentIndex + 1 put: Color
> transparent
>         ].
>         f colors: thisImageColorTable.
>         ^ f
>   !
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.squeakfoundation.org/pipermail/squeak-dev/attachments/20140722/81f1ea08/attachment.htm