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@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@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@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