And in PNGReadWriter>>nextImage we should remove stream reset and stream binary.
This probably goes for the other image readers as well. > On 15 Mar 2018, at 13:38, Sven Van Caekenberghe <s...@stfx.eu> wrote: > > Yeah, right, good catch. > > But what is this nonsense: > > PNGReadWriter>>on: aStream > (stream := aStream) reset. > stream binary > "Note that 'reset' makes a file be text. Must do this after." > > Of course you can only read an image from a binary stream, there should be > nothing else there but > > stream := aStream > >> On 15 Mar 2018, at 13:31, Alistair Grant <akgrant0...@gmail.com> wrote: >> >> Also: >> >> 'apicture.png' asFileReference inspect. >> >> Will raise a "image format not recognised" because: >> >> - AbstractFileReference>>binaryReadStreamDo: ultimately uses a >> ZnBufferedReadStream >> - ImageReadWriter>>on: sends #reset to the stream >> >> and ZnBufferedReadStream doesn't implement #reset. >> >> My guess is that ZnBufferedReadStream should implement #reset (it has >> #position:). But I'm not familiar with the current thinking on stream >> rationalisation. >> >> I'll raise some fogbugz issues a bit later. >> >> Image downloaded today: >> >> Pharo 7.0 >> Build information: >> Pharo-7.0+alpha.build.696.sha.a26248bb90ee8dfba131f2f4d71ba2bbbad5d00d >> (64 Bit) >> >> >> >> Thanks, >> Alistair >> >> >> On 15 March 2018 at 13:08, Sven Van Caekenberghe <s...@stfx.eu> wrote: >>> BTW, for the record, we should deprecate Base64MimeConverter, and >>> remove/change the following convenience methods: >>> >>> Remove >>> >>> String>>#base64Encoded. >>> >>> Change >>> >>> ByteArray>>#base64Encoded >>> "Encode the receiver using Base64, see ZnBase64Encoder" >>> >>> ^ ZnBase64Encoder new encode: self >>> >>> String>>#base64Decoded >>> "Decode the receiver using Base64, see ZnBase64Encoder" >>> >>> ^ ZnBase64Encoder new decode: self >>> >>> The thing is, Base64 is defined as going from bytes -> characters and vice >>> versa. Not as going from characters -> characters. For that you need to use >>> an explicit character encoder as in ZnUtils class>>#decodeBase64: and >>> ZnUtils>>#encodeBase64: >>> >>> So yes, this would be a breaking change, but an easy one to follow since >>> Base64 is really very simple as a concept. >>> >>>> On 15 Mar 2018, at 12:47, Sven Van Caekenberghe <s...@stfx.eu> wrote: >>>> >>>> >>>> >>>>> On 15 Mar 2018, at 12:28, Alistair Grant <akgrant0...@gmail.com> wrote: >>>>> >>>>> Hi Guille & Pavel, >>>>> >>>>> On 6 Feb Guille changed Base64MimeConverter class>>mimeDecodeToBytes: >>>>> so that it doesn't reset the dataStream position back to the start of >>>>> the stream. This breaks Pavel's PlayingCard, part of FreeCell (you >>>>> can see that I only load important stuff :-)). >>>>> >>>>> PlayingCard class>>initialize >>>>> "PlayingCard initialize" >>>>> | forms f | >>>>> "Read the stored forms from mime-encoded data in imageData." >>>>> f := Base64MimeConverter mimeDecodeToBytes: (ReadStream on: self >>>>> imageData). >>>>> forms := OrderedCollection new. >>>>> f next = 2 ifFalse: [self error: 'corrupted imageData']. >>>>> [f atEnd] whileFalse: [forms add: (Form new readFrom: f)]. >>>>> ... >>>>> >>>>> >>>>> Previously, f would be at the start of the stream (which makes sense >>>>> given this usage), but now it is at the end of the stream, so returns >>>>> nil and "f next = 2" fails. >>>>> >>>>> Guille, can you explain the change. The commit log just says "make tests >>>>> run". >>>>> >>>>> Note that a lot of users of this won't fail as they just call >>>>> #contents of the stream after #mimeDecodeToBytes:, which works >>>>> regardless of the current position. >>>> >>>> So doing >>>> >>>> f reset >>>> >>>> at the caller site would fix it then ? >>>> >>>> Note that there is the newer ZnBase64Encoder that can be used as follows: >>>> >>>> f := (ZnBase64Encoder new decode: self imageData) readStream. >>>> >>>> Sven >>> >>> >> >
