[Pkg] The Trunk: Graphics-nice.296.mcz

commits at source.squeak.org commits at source.squeak.org
Mon Jul 21 23:30:20 UTC 2014

Nicolas Cellier uploaded a new version of Graphics to project The Trunk:

==================== 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?
I'm quite sure those reset has something to do with following usage:
    (ReadWriteStream with: 'foo') next -> nil
    (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') -----
  	| 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].
  				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') -----
+ 	"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

More information about the Packages mailing list