The Inbox: EToys-cbc.293.mcz

Hi Chris,

thanks for looking into this! ExternalForm is not used by Etoys, so it should just be removed along with the test. I guess it's a leftover from Balloon3D.

For the FFI package, you could submit a fixed version to the Inbox and any core developer can move it to the FFI repo.

- Bert -

Actually, Elliot, did you want to keep it in the image?  I was removing it pursuant to Bert's suggestion.

After removing it myself, and then doing a Smalltalk garbageCollect, it is no longer anywhere in the image (or Undeclared, either).  (Although BlockContext is, but that's another issue.)

I have added a new package for it if you want to keep it around.  We went here because the ExternalForm was removed from FFI and put into EToys, and then Bert said it wasn't used in EToys (and it isn't).

Now in package: http://source.squeak.org/inbox/GraphicsExternal-cbc.1.mcz

On Thu, Apr 20, 2017 at 4:57 AM, Chris Cunningham <[hidden email]> wrote:

Actually, Elliot, did you want to keep it in the image?  I was removing it pursuant to Bert's suggestion.

After removing it myself, and then doing a Smalltalk garbageCollect, it is no longer anywhere in the image (or Undeclared, either).  (Although BlockContext is, but that's another issue.)

I have added a new package for it if you want to keep it around.  We went here because the ExternalForm was removed from FFI and put into EToys, and then Bert said it wasn't used in EToys (and it isn't).

Now in package: http://source.squeak.org/inbox/GraphicsExternal-cbc.1.mcz

Nice. I think you missed some extension methods, e.g. #isExternalForm,  #isFillAccelerated:for:, #isBltAccelerated:for:, #displayScreen in class Form. If you move these into a *graphicsexternal category, they will become part of that package. 

- Bert - 

On Thu, Apr 20, 2017 at 8:34 AM, Bert Freudenberg <[hidden email]> wrote:

On Thu, Apr 20, 2017 at 4:57 AM, Chris Cunningham <[hidden email]> wrote:

Actually, Elliot, did you want to keep it in the image?  I was removing it pursuant to Bert's suggestion.

After removing it myself, and then doing a Smalltalk garbageCollect, it is no longer anywhere in the image (or Undeclared, either).  (Although BlockContext is, but that's another issue.)

I have added a new package for it if you want to keep it around.  We went here because the ExternalForm was removed from FFI and put into EToys, and then Bert said it wasn't used in EToys (and it isn't).

Now in package: http://source.squeak.org/inbox/GraphicsExternal-cbc.1.mcz

Nice. I think you missed some extension methods, e.g. #isExternalForm,  #isFillAccelerated:for:, #isBltAccelerated:for:, #displayScreen in class Form. If you move these into a *graphicsexternal category, they will become part of that package. 

- Bert - 

Ok.

I can see that I probably should have noticed #isExternalForm, but I wouldn't have had a chance to find the other ones. 

Also, these weren't part of the Etoys nor FFI packages - they were in the base Graphics package.

That said, I'll moved them over.

Anything else lying around?

Yeah, isn't it fun to untangle a monolithic system? ;)

Anything else lying around?

There is also the stuff with hasNonStandardPalette and convertARGB etc which is only used if there are external forms, but that seems a bit harder to remove because the hooks need to be there. I think it's fine to leave that as it is, unless you notice something that's easily removable.

- Bert -

Hi Chris,

On Apr 19, 2017, at 7:57 PM, Chris Cunningham <[hidden email]> wrote:

Actually, Elliot, did you want to keep it in the image?  I was removing it pursuant to Bert's suggestion.

No, which is why I suggested putting it in some new package.  It shouldn't get lost; someone may find it useful, and the Pharo folks are usually by the vm father nationality to support Athens.

After removing it myself, and then doing a Smalltalk garbageCollect, it is no longer anywhere in the image (or Undeclared, either).  (Although BlockContext is, but that's another issue.)

I have added a new package for it if you want to keep it around.  We went here because the ExternalForm was removed from FFI and put into EToys, and then Bert said it wasn't used in EToys (and it isn't).

Thanks 

Now in package: http://source.squeak.org/inbox/GraphicsExternal-cbc.1.mcz

Great!

Thanks,

cbc

On Wed, Apr 19, 2017 at 4:47 PM, Eliot Miranda <[hidden email]> wrote:

Hi All,

    can we please do something like put it in a new package called GraphicsExternal or FFI-External?  Or put it in FFI-Examples?  It's orphaned in my work image at the moment :-(.

On Mon, Apr 17, 2017 at 1:01 PM, Chris Cunningham <[hidden email]> wrote:

Hi.

there are now two options in the inbox.

First move http://source.squeak.org/inbox/FFI-Tests-cbc.9.mcz to Trunk to get the test for ExternalForm out of FFI (since ExternalForm is no longer in FFI).

Then, either:

1> move http://source.squeak.org/inbox/EToys-cbc.293.mcz to trunk (to keep ExternalForm in the image, if desired, but add the test to EToys), or

2> move http://source.squeak.org/inbox/EToys-cbc.294.mcz to trunk, to competely remove ExternalForm and ExternalScreen from Squeak, since they are no longer used (also doesn't has the test removed).

Thanks,

cbc

On Mon, Apr 17, 2017 at 10:12 AM, Bert Freudenberg <[hidden email]> wrote:

Hi Chris,

thanks for looking into this! ExternalForm is not used by Etoys, so it should just be removed along with the test. I guess it's a leftover from Balloon3D.

For the FFI package, you could submit a fixed version to the Inbox and any core developer can move it to the FFI repo.

- Bert -

<snip> 

--

_,,,^..^,,,_

best, Eliot

On Thu, Apr 20, 2017 at 9:32 AM, Bert Freudenberg <[hidden email]> wrote:

<snip/>

Yeah, isn't it fun to untangle a monolithic system? ;)

Not bad if you don't try and change things... 

 

<snip/> 

Anything else lying around?

There is also the stuff with hasNonStandardPalette and convertARGB etc which is only used if there are external forms, but that seems a bit harder to remove because the hooks need to be there. I think it's fine to leave that as it is, unless you notice something that's easily removable.

There is a new version in the inbox with those 4 method moved.

The rest are doable.  THe right way to do them, I think, is to remove most of these methods completely from Form. Then, adjust the methods that call them (in Form) to do the standard form code; and have override varients in ExternalForm (and/or ExternalScreen) that does the very specific alternates - or delegates to Form if it should.  Those dependant methods seem to have a lot of assumptions about what the other variants might be.

I might get to this next week at some point, but I won't promise anything.

On Apr 21, 2017 11:23 AM, "Bert Freudenberg" <[hidden email]> wrote:

I think we should leave this as is. The hooks need to stay in place if we still want to support external forms.

We might want to amend the comment in #hasNonStandardPalette with some mention of the new external graphics package.

 - Bert -

I became interested in this because a test failed, so I can stop when it is deemed right to do so. That said:

The current methods calling the methods you mention will look very weird without the ExternalForm loaded. There are branches that make no sense with that package removed. I did 3+ years from now if someone needs to work on Form and especially those methods they might be tempted to just remove them and then we are much worse off. 

What I'm suggesting is to factor out ExternalForm versions of those methods into ExternalForm and ExternalScreen where they will make sense to someone looking over it in the future. In the process the methods in Form should also get simple and more understandable (although there is a lot of but twiddling left in them).  As a bonus I would leave a note in the Form comment about the existence of ExternalForm in case someone was looking for such a thing. 

Yes this approach can also for over time - but at least ExternalForm will be consistent with itself.  And it might not for as fast asvleaving the mixed methods in there now. The only good way not to for is to put the classes back in, really.

On Sat, Apr 22, 2017 at 3:49 AM, Chris Cunningham <[hidden email]> wrote:

On Apr 21, 2017 11:23 AM, "Bert Freudenberg" <[hidden email]> wrote:

I think we should leave this as is. The hooks need to stay in place if we still want to support external forms.

We might want to amend the comment in #hasNonStandardPalette with some mention of the new external graphics package.

 - Bert -

I became interested in this because a test failed, so I can stop when it is deemed right to do so. That said:

The current methods calling the methods you mention will look very weird without the ExternalForm loaded. There are branches that make no sense with that package removed. I did 3+ years from now if someone needs to work on Form and especially those methods they might be tempted to just remove them and then we are much worse off. 

What I'm suggesting is to factor out ExternalForm versions of those methods into ExternalForm and ExternalScreen where they will make sense to someone looking over it in the future. In the process the methods in Form should also get simple and more understandable (although there is a lot of but twiddling left in them).  As a bonus I would leave a note in the Form comment about the existence of ExternalForm in case someone was looking for such a thing. 

Ah, I see what you mean. Basically the non-standard palette behavior would only be implemented in ExternalForm and ExternalScreen.

This makes sense, except that it would lead to lots of code duplication. The closest common ancestor of ExternalForm and ExternalScreen is Form, and I'm certain that's why Andreas put the code for dealing with non-standard palette code in Form. (I guess nowadays this could be solved with a NonStandardPaletteTrait that would be used by both ExternalForm and ExternalScreen, but I'm not sure this is preferable to just leaving it in Form).

 

Yes this approach can also for over time - but at least ExternalForm will be consistent with itself.  And it might not for as fast asvleaving the mixed methods in there now. The only good way not to for is to put the classes back in, really.

It's not about speed, it's about being able to mix regular and external forms. But if you can refactor it so it still works and all the code is preserved, awesome!

One more thing: 

It just occurred to me that the right place to put this is the Balloon3D repository:

http://www.squeaksource.com/Balloon3D.html

I just added you as a developer. 

I also noticed that there is already a Graphics-External package in there. I have not compared it to yours, but it might be interesting to check. 

Again, thanks for doing this :)

- Bert -