<div dir="ltr"><br><div class="gmail_extra"><br><br><div class="gmail_quote">2014-07-22 1:29 GMT+02:00 <span dir="ltr"><<a href="mailto:commits@source.squeak.org" target="_blank">commits@source.squeak.org</a>></span>:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Nicolas Cellier uploaded a new version of Graphics to project The Trunk:<br>
<a href="http://source.squeak.org/trunk/Graphics-nice.296.mcz" target="_blank">http://source.squeak.org/trunk/Graphics-nice.296.mcz</a><br>
<br>
==================== Summary ====================<br>
<br>
Name: Graphics-nice.296<br>
Author: nice<br>
Time: 22 July 2014, 1:28:48.356 am<br>
UUID: 3f6d869e-9929-4e08-b137-aae22dd3eee6<br>
Ancestors: Graphics-nice.295<br>
<br>
Don't try to accelerate GIF reading by copying the stream in memory.<br>
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.<br>
<br>
Especially when the patch is testing that the stream is in memory with a class identity to ReadWriteStream!!!<br>
Frankly, how do we know that it's not a RWBinaryOrTextStream ;)<br>
<br>
So why reading an image with a ReadWriteStream?<br>
Oh, sure, this is in ImageReadWriter, so we may need a ReadWriteStream?<br>
No, no, and no!<br>
This thing will either read or write but NEVER read and write.<br>
One simple role, one simple class, no UberPotentSwissKnifeStream with UberComplexStateToManage (readPosition + writePosition + readLimit + writeLimit + readBuffer + writeBuffer + readWriteBufferInteraction).<br>
<br>
What's coming next?<br>
Easy to guess, see what (ImageReadWriter>>on:) does frivously to our stream:<br>
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?<br></blockquote><div><br></div><div>the answer is summarized there:<br>
<br>readNativeResourceFrom: byteStream<br> | img aStream |<br> (byteStream isKindOf: FileStream) ifTrue:[<br> "Ugly, but ImageReadWriter will send #reset which is implemented as #reopen and we may not be able to do so."<br>
aStream := RWBinaryOrTextStream with: byteStream contents.<br> ] ifFalse:[<br> aStream := byteStream.<br> ].<br><br></div><div>Ugly says it all.<br><br></div><div> <br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
I'm quite sure those reset has something to do with following usage:<br>
(ReadWriteStream with: 'foo') next -> nil<br>
but:<br>
(ReadWriteStream with: 'foo') reset; next -> $f<br>
Pure Read/Write madness.<br>
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.<br>
<br>
=============== Diff against Graphics-nice.295 ===============<br>
<br>
Item was changed:<br>
----- Method: AnimatedGIFReadWriter>>allImages (in category 'accessing') -----<br>
allImages<br>
| body colorTable |<br>
- stream class == ReadWriteStream ifFalse: [<br>
- stream binary.<br>
- self on: (ReadWriteStream with: (stream contentsOfEntireFile))].<br>
localColorTable := nil.<br>
forms := OrderedCollection new.<br>
delays := OrderedCollection new.<br>
comments := OrderedCollection new.<br>
self readHeader.<br>
[(body := self readBody) == nil]<br>
whileFalse: [colorTable := localColorTable<br>
ifNil: [colorPalette].<br>
transparentIndex<br>
ifNotNil: [transparentIndex + 1 > colorTable size<br>
ifTrue: [colorTable := colorTable forceTo: transparentIndex + 1 paddingWith: Color white].<br>
colorTable at: transparentIndex + 1 put: Color transparent].<br>
body colors: colorTable.<br>
forms add: body.<br>
delays add: delay].<br>
^ forms!<br>
<br>
Item was changed:<br>
----- Method: GIFReadWriter>>nextImage (in category 'accessing') -----<br>
nextImage<br>
+ "Read in the next GIF image from the stream."<br>
- "Read in the next GIF image from the stream. Read it all into<br>
- memory first for speed."<br>
<br>
| f thisImageColorTable |<br>
- stream class == ReadWriteStream ifFalse: [<br>
- stream binary.<br>
- self on: (ReadWriteStream with: (stream contentsOfEntireFile))].<br>
<br>
localColorTable := nil.<br>
self readHeader.<br>
f := self readBody.<br>
self close.<br>
f == nil ifTrue: [^ self error: 'corrupt GIF file'].<br>
<br>
thisImageColorTable := localColorTable ifNil: [colorPalette].<br>
transparentIndex ifNotNil: [<br>
transparentIndex + 1 > thisImageColorTable size ifTrue: [<br>
thisImageColorTable := thisImageColorTable<br>
forceTo: transparentIndex + 1<br>
paddingWith: Color white<br>
].<br>
thisImageColorTable at: transparentIndex + 1 put: Color transparent<br>
].<br>
f colors: thisImageColorTable.<br>
^ f<br>
!<br>
<br>
<br>
</blockquote></div><br></div></div>