[Vm-dev] Support for reading and writing JPEGs to/from -32, -16, -8 and 8 bit deep forms on V3

David T. Lewis lewis at mail.msen.com
Fri May 20 02:05:39 UTC 2016


Hi Laura,

Thanks very much for posting this. I took a quick look at it and will
try to follow up later.

To clarify one point: Eliot is using the word "horrible" to refer to
some coding practices in the existing JPEGReadWriter2Plugin, and it does
not mean that your patches are "horrible".


Eliot,

Indeed, those giant blocks of #cCode are bad, and somebody (tm) should
do something about it. It may not be obvious from looking at the change
set, but Laura's updates are are patches against that existing code.
So your suggestion is good - either rewrite those portions in proper
Smalltalk slang, or just move them out to C code in the platforms support.

I think that this is probably beyond the scope of what Laura is trying to
do here, although I would encourage anyone with an interest in improving
the plugins to take a look at reworking the #cCode portions of
JPEGReadWriter2Plugin.

Dave




On Thu, May 19, 2016 at 04:29:15PM -0700, Eliot Miranda wrote:
>  
> Hi Laura,
> 
>     this isn't your fault, but looking at the body of the plugin methods,
> the code is horrible.  It's essentially 100% C.  The code should really be
> written in C files, not in Smalltalk methods, following the pattern of
> separating plugin code into a Smalltalk plugin larger calling platform
> functions written in a C layer.  For example, look at the REPlugin which
> does regular expressions.  The source to the machinery is in
> platforms/Cross/plugins/RePlugin.  The plugin methods, e.g.
> REPlugin>>primPCREExecfromto, do marshalling of Smalltalk objects into C
> parameters and then invoke function implemented in
> platforms/Cross/plugins/RePlugin, e.g. pcre_exec.  This approach means that
> there's onlyas much cCode:inSmalltalk: as necessary.  This is far nicer
> code to read and fix.
> 
> Do you have energy to rewrite the JPEG plain code to follow this pattern?
> If so, let me encourage you.  It will improve the code hugely.
> 
> On Thu, May 19, 2016 at 7:06 AM, Laura Perez Cerrato <
> lauraperezcerrato at gmail.com> wrote:
> 
> >
> > Elliot, Dave,
> >
> > Thanks for your answers! Here go both the changesets requested. The
> > JPEGReadWriter2Plugin changeset was generated using a Squeak Spur
> > trunk image, whilst the Graphics changeset was generated using the
> > latest stable version of Squeak non-Spur (4.6-15102). However, It
> > should also work well with a Squeak Spur trunk image, though I haven't
> > tested it's functioning there thoroughly. I haven't yet implemented a
> > graceful way for the new primitive to fail if the new version of the
> > plugin is absent, so keep that in mind.
> >
> > I'm currently working on handling non-even width 16-bit Forms and
> > non-0 modulo 4 width 8 bit Forms, so I'll hopefully send that to you
> > too within this week.
> >
> > -Laura Perez Cerrato
> >
> >
> > On 18 May 2016 at 22:00, David T. Lewis <lewis at mail.msen.com> wrote:
> > >
> > > On Wed, May 18, 2016 at 03:56:11PM -0700, Eliot Miranda wrote:
> > >>
> > >> Hi Laura,
> > >>
> > >> > On May 18, 2016, at 1:04 PM, Laura Perez Cerrato <
> > lauraperezcerrato at gmail.com> wrote:
> > >> >
> > >> >
> > >> > Hello, everyone,
> > >> >
> > >> > During the past few days I've been working on adding support for
> > >> > reading and writing reading and writing JPEGs to/from -32, -16, -8
> > >> > grayscale and 8 grayscale bit deep forms to JPEGReadWriter2Plugin for
> > >> > the Squeak Cog V3 VM. Just chiming in to tell you that the work is
> > >> > done and I'd like to share it with you all. :) I believe this changes
> > >> > should work on Spur VMs too, but haven't tested thoroughly enough to
> > >> > ensure that.
> > >> >
> > >> > How do you handle contributions to the codebase? I assume I'd have to
> > >> > upload the corresponding changeset to the repository but I'm not sure
> > >> > if there's anything I should do before.
> > >> >
> > >> > I have also been working on a few changes to the latest non-Spur
> > >> > version of Squeak in order to take advantage of the changes to the
> > >> > plugin but, since I added a primitive to it and these changes use it,
> > >> > I think it's prudent to ask you too how those contributions are
> > >> > handled. Should I refer to the Squeak-dev mailing list?
> > >>
> > >> This is indeed a problem.  Talking for myself, until VMs have been
> > built that include the code and are generally available one can't really
> > deploy the behaviour to trunk.  But again waiting for the next release
> > feels far too slow.
> > >>
> > >> One approach is to make the behaviour optional, depending on what the
> > VM provides, eg checking for primitive failure or a plugin version number,
> > or simply making the code fail gracefully.
> > >>
> > >> Another approach is to provide the functionality in an extension
> > package, and merging that extension into trunk at the next release.
> > >>
> > >> Other suggestions folks?
> > >>
> > >
> > > We should take a look at the image side code too. Laura, perhaps you can
> > > post a change set for that also?
> > >
> > > Very often a new optional primitive can be handled simply by providing
> > > some suitable fallback code. That provides a smooth transition, because
> > > the new feature is immediately available and the system continues to work
> > > for people who do not yet have the new primitive in their VM.
> > >
> > > Dave
> > >
> >
> >
> 
> 
> -- 
> _,,,^..^,,,_
> best, Eliot



More information about the Vm-dev mailing list