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?
Thanks in advance,
-Laura Perez Cerrato
Hi Laura,
I expect that if your plugin changes are working on non-Spur, then they probably will work on Spur also. If you have a change set, can you post it to the list here?
Thanks a lot! Dave
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?
Thanks in advance,
-Laura Perez Cerrato
Laura,
Wonderful! Thanks! As David says, please post a change set of your work here.
_,,,^..^,,,_ (phone)
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?
Thanks in advance,
-Laura Perez Cerrato
Hi Laura,
the way we like to handle submissions is for people to commit the entire package. Right now we don't have an inbox for VMMaker, so to contribute one has to "earn one's spurs". So for now emailing a change set will work. But soon enough we'll discuss making you a full committee so that in future you'd merely commit to the VMMaker repository. Forgive the vetting process. We, the existing committers, have all been through the vetting process so it wouldn't be fair to allow you in immediately ;-)
_,,,^..^,,,_ (phone)
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?
Thanks in advance,
-Laura Perez Cerrato
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?
Thanks in advance,
-Laura Perez Cerrato
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
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
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
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@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
Elliot, Dave,
Thanks for your feedback! I didn't realise the differences between the coding practices of JPEGReadWriter2Plugin and other plugins since I didn't look at the code of the others. I'll try to get the code of JPEGReadWriter2Plugin to bear more resemblance to the rest of the codebase. I've taken a brief look at RePlugin's code and the way the C code should be placed into external files is still a bit confusing to me. Any guidance in this aspect would be greatly appreciated.
As I mentioned earlier, I've been working on handling non-even width 16-bit Forms and non-0 modulo 4 width 8 bit Forms. Here are two changesets with the implementation of that feature.
Cheers! -Laura Perez Cerrato
On 19 May 2016 at 23:05, David T. Lewis lewis@mail.msen.com wrote:
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@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
Laura, since you’re digging into this area, you may get some inspiration from something I posted a while back
Somebody on the Pi forums has been looking into the jpglib stuff and concluded that there is a simple fix to allow use of the normal linux libjpeg. This would be nice if true.
I’m currently swamped but anyone interested can look at https://www.raspberrypi.org/forums/viewtopic.php?p=934869#p934869 and the code and who konws what may happen?
It would be really nice to be able to drop keeping our own copy of all that code and return to using the ‘normal’ library.
tim -- tim Rowledge; tim@rowledge.org; http://www.rowledge.org/tim Mommy! The cursor's winking at me!
Excellent, thanks Laura!
Dave
Elliot, Dave,
Thanks for your feedback! I didn't realise the differences between the coding practices of JPEGReadWriter2Plugin and other plugins since I didn't look at the code of the others. I'll try to get the code of JPEGReadWriter2Plugin to bear more resemblance to the rest of the codebase. I've taken a brief look at RePlugin's code and the way the C code should be placed into external files is still a bit confusing to me. Any guidance in this aspect would be greatly appreciated.
As I mentioned earlier, I've been working on handling non-even width 16-bit Forms and non-0 modulo 4 width 8 bit Forms. Here are two changesets with the implementation of that feature.
Cheers! -Laura Perez Cerrato
On 19 May 2016 at 23:05, David T. Lewis lewis@mail.msen.com wrote:
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 pluginmethods, 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
Hi Laura,
On Thu, May 26, 2016 at 7:00 AM, Laura Perez Cerrato < lauraperezcerrato@gmail.com> wrote:
Elliot, Dave,
Thanks for your feedback! I didn't realise the differences between the coding practices of JPEGReadWriter2Plugin and other plugins since I didn't look at the code of the others. I'll try to get the code of JPEGReadWriter2Plugin to bear more resemblance to the rest of the codebase. I've taken a brief look at RePlugin's code and the way the C code should be placed into external files is still a bit confusing to me. Any guidance in this aspect would be greatly appreciated.
So the scheme is a) the generated C source for the Smalltalk plugin class is generated in src/plugins/PluginName/PluginName.c, and it includes a PluginName.h b) the PluginName.h lives in platforms/Cross/plugins/PluginName/PluginName.h. It should define the functions and types exported by any C sport for the PluginName.c file to use. c) if the C support code is cross-platform it should live in platforms/Cross/plugins/PluginName/sqPluginName.c, which also includes PluginName.c. That's the case here, so refactor the bodies of all plugin methods so that the C code tat is in cCode: '...here...' strings gets moved to functions in platforms/Cross/plugins/PluginName/sqPluginName.c. These functions can be called anything you like but there's a convention to prepend "sq" or "io". For example in the FilePlugin names like sqFileFlush. "io" tends to be for more GUI and OS related functionality. d) if the C code is not cross p;at form, or part of it is not cross platform, that code goes in platforms/platformName/plugins/PluginName/sqPlatPluginName.c, e.g. platforms/win32/plugins/UUIDPlugin/sqWin32UUID.c
As I mentioned earlier, I've been working on handling non-even width 16-bit Forms and non-0 modulo 4 width 8 bit Forms. Here are two changesets with the implementation of that feature.
Thanks!!
Cheers! -Laura Perez Cerrato
On 19 May 2016 at 23:05, David T. Lewis lewis@mail.msen.com wrote:
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 pluginmethods,
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
Elliot,
Thanks for the detailed instructions! They were really helpful. I managed to refactor the plugin to be more readable. Here's the corresponding changeset and C code file. which should be placed in platforms/Cross/plugins/JPEGReadWriter2Plugin
Cheers! -Laura Perez Cerrato
On 26 May 2016 at 19:09, Eliot Miranda eliot.miranda@gmail.com wrote:
Hi Laura,
On Thu, May 26, 2016 at 7:00 AM, Laura Perez Cerrato lauraperezcerrato@gmail.com wrote:
Elliot, Dave,
Thanks for your feedback! I didn't realise the differences between the coding practices of JPEGReadWriter2Plugin and other plugins since I didn't look at the code of the others. I'll try to get the code of JPEGReadWriter2Plugin to bear more resemblance to the rest of the codebase. I've taken a brief look at RePlugin's code and the way the C code should be placed into external files is still a bit confusing to me. Any guidance in this aspect would be greatly appreciated.
So the scheme is a) the generated C source for the Smalltalk plugin class is generated in src/plugins/PluginName/PluginName.c, and it includes a PluginName.h b) the PluginName.h lives in platforms/Cross/plugins/PluginName/PluginName.h. It should define the functions and types exported by any C sport for the PluginName.c file to use. c) if the C support code is cross-platform it should live in platforms/Cross/plugins/PluginName/sqPluginName.c, which also includes PluginName.c. That's the case here, so refactor the bodies of all plugin methods so that the C code tat is in cCode: '...here...' strings gets moved to functions in platforms/Cross/plugins/PluginName/sqPluginName.c. These functions can be called anything you like but there's a convention to prepend "sq" or "io". For example in the FilePlugin names like sqFileFlush. "io" tends to be for more GUI and OS related functionality. d) if the C code is not cross p;at form, or part of it is not cross platform, that code goes in platforms/platformName/plugins/PluginName/sqPlatPluginName.c, e.g. platforms/win32/plugins/UUIDPlugin/sqWin32UUID.c
As I mentioned earlier, I've been working on handling non-even width 16-bit Forms and non-0 modulo 4 width 8 bit Forms. Here are two changesets with the implementation of that feature.
Thanks!!
Cheers! -Laura Perez Cerrato
On 19 May 2016 at 23:05, David T. Lewis lewis@mail.msen.com wrote:
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 pluginmethods, 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
-- _,,,^..^,,,_ best, Eliot
vm-dev@lists.squeakfoundation.org
-
David T. Lewis -
Eliot Miranda -
Laura Perez Cerrato -
tim Rowledge