Quantcast

The Inbox: EToys-cbc.293.mcz

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
18 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

The Inbox: EToys-cbc.293.mcz

commits-2
A new version of EToys was added to project The Inbox:
http://source.squeak.org/inbox/EToys-cbc.293.mcz

==================== Summary ====================

Name: EToys-cbc.293
Author: cbc
Time: 17 April 2017, 9:44:25.306187 am
UUID: 7f27f8f3-1ad3-9d4b-a359-1b085c98e5c2
Ancestors: EToys-nice.292

Moved ExternalForm test from FFI package here - since the ExternalForm is now in this package.
Also, fixed test.

=============== Diff against EToys-nice.292 ===============

Item was added:
+ TestCase subclass: #EtoysExternalFormTest
+ instanceVariableNames: ''
+ classVariableNames: ''
+ poolDictionaries: ''
+ category: 'Etoys-Tests'!

Item was added:
+ ----- Method: EtoysExternalFormTest>>testBlitToAndFromExternalForm (in category 'as yet unclassified') -----
+ testBlitToAndFromExternalForm
+ "Ensure that we can blit to/from all 4 permutatations of Form and ExternalForm."
+ | source external1 external2 destination |
+ source := Cursor wait asCursorForm asFormOfDepth: 32.
+ destination := Form extent: source extent depth: 32.
+ external1 := ExternalForm extent: source extent depth: 32.
+ external2 := ExternalForm extent: source extent depth: 32.
+ self shouldnt: [source bits = destination bits].
+ source displayOn: external1.
+ external1 displayOn: external2.
+ external2 displayOn: destination.
+ self should: [source bits = destination bits].!


cbc
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: The Inbox: EToys-cbc.293.mcz

cbc
Hi.

If someone could also either give me access to source.squeag.org/FFI or remove this test from that repository as well, I'd appreciate it.
This test was failing in the FFI test suite because there was no longer a need to send #allocateSpace on ExternalForm.  It is (was?) also incorrectly located - test in FFI, form now in EToys.

This should fix moving the test and fixing it (if moved to trunk); the final piece is to remove it from FFI.

Thanks,
-cbc

On Mon, Apr 17, 2017 at 9:44 AM, <[hidden email]> wrote:
A new version of EToys was added to project The Inbox:
http://source.squeak.org/inbox/EToys-cbc.293.mcz

==================== Summary ====================

Name: EToys-cbc.293
Author: cbc
Time: 17 April 2017, 9:44:25.306187 am
UUID: 7f27f8f3-1ad3-9d4b-a359-1b085c98e5c2
Ancestors: EToys-nice.292

Moved ExternalForm test from FFI package here - since the ExternalForm is now in this package.
Also, fixed test.

=============== Diff against EToys-nice.292 ===============

Item was added:
+ TestCase subclass: #EtoysExternalFormTest
+       instanceVariableNames: ''
+       classVariableNames: ''
+       poolDictionaries: ''
+       category: 'Etoys-Tests'!

Item was added:
+ ----- Method: EtoysExternalFormTest>>testBlitToAndFromExternalForm (in category 'as yet unclassified') -----
+ testBlitToAndFromExternalForm
+       "Ensure that we can blit to/from all 4 permutatations of Form and ExternalForm."
+       | source external1 external2 destination |
+       source := Cursor wait asCursorForm asFormOfDepth: 32.
+       destination := Form extent: source extent depth: 32.
+       external1 := ExternalForm extent: source extent depth: 32.
+       external2 := ExternalForm extent: source extent depth: 32.
+       self shouldnt: [source bits = destination bits].
+       source displayOn: external1.
+       external1 displayOn: external2.
+       external2 displayOn: destination.
+       self should: [source bits = destination bits].!





Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: The Inbox: EToys-cbc.293.mcz

Bert Freudenberg
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 -

On Mon, Apr 17, 2017 at 6:49 PM, Chris Cunningham <[hidden email]> wrote:
Hi.

If someone could also either give me access to source.squeag.org/FFI or remove this test from that repository as well, I'd appreciate it.
This test was failing in the FFI test suite because there was no longer a need to send #allocateSpace on ExternalForm.  It is (was?) also incorrectly located - test in FFI, form now in EToys.

This should fix moving the test and fixing it (if moved to trunk); the final piece is to remove it from FFI.

Thanks,
-cbc

On Mon, Apr 17, 2017 at 9:44 AM, <[hidden email]> wrote:
A new version of EToys was added to project The Inbox:
http://source.squeak.org/inbox/EToys-cbc.293.mcz

==================== Summary ====================

Name: EToys-cbc.293
Author: cbc
Time: 17 April 2017, 9:44:25.306187 am
UUID: 7f27f8f3-1ad3-9d4b-a359-1b085c98e5c2
Ancestors: EToys-nice.292

Moved ExternalForm test from FFI package here - since the ExternalForm is now in this package.
Also, fixed test.

=============== Diff against EToys-nice.292 ===============

Item was added:
+ TestCase subclass: #EtoysExternalFormTest
+       instanceVariableNames: ''
+       classVariableNames: ''
+       poolDictionaries: ''
+       category: 'Etoys-Tests'!

Item was added:
+ ----- Method: EtoysExternalFormTest>>testBlitToAndFromExternalForm (in category 'as yet unclassified') -----
+ testBlitToAndFromExternalForm
+       "Ensure that we can blit to/from all 4 permutatations of Form and ExternalForm."
+       | source external1 external2 destination |
+       source := Cursor wait asCursorForm asFormOfDepth: 32.
+       destination := Form extent: source extent depth: 32.
+       external1 := ExternalForm extent: source extent depth: 32.
+       external2 := ExternalForm extent: source extent depth: 32.
+       self shouldnt: [source bits = destination bits].
+       source displayOn: external1.
+       external1 displayOn: external2.
+       external2 displayOn: destination.
+       self should: [source bits = destination bits].!









cbc
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: The Inbox: EToys-cbc.293.mcz

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 -

I'll submit that, then.  Wasn't sure where to send it.

Thanks,
cbc 


cbc
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: The Inbox: EToys-cbc.293.mcz

cbc
In reply to this post by Bert Freudenberg
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> 


Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: The Inbox: EToys-cbc.293.mcz

Eliot Miranda-2
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


cbc
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: The Inbox: EToys-cbc.293.mcz

cbc
Sorry about that. If no-one beats me to it I will do so when I get to a computer (in about 2 hours)  

On Apr 19, 2017 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





cbc
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: The Inbox: EToys-cbc.293.mcz

cbc
In reply to this post by Eliot Miranda-2
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).


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






Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: The Inbox: EToys-cbc.293.mcz

Eliot Miranda-2
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 


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







Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: The Inbox: EToys-cbc.293.mcz

Bert Freudenberg
In reply to this post by cbc
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).


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 - 


cbc
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: The Inbox: EToys-cbc.293.mcz

cbc

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).


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?

-cbc



Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: The Inbox: EToys-cbc.293.mcz

Bert Freudenberg
On Thu, Apr 20, 2017 at 5:54 PM, Chris Cunningham <[hidden email]> wrote:

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).


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. 

I just gave the code a cursory look ... these are implemented in ExternalForm but there is a base implementation.
 
Also, these weren't part of the Etoys nor FFI packages - they were in the base Graphics package.

Yeah, isn't it fun to untangle a monolithic system? ;)
 
That said, I'll moved them over.

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 -


cbc
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: The Inbox: EToys-cbc.293.mcz

cbc

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.

-cbc

- Bert -






Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: The Inbox: EToys-cbc.293.mcz

Eliot Miranda-2
In reply to this post by Eliot Miranda-2


On Wed, Apr 19, 2017 at 8:22 PM, Eliot Miranda <[hidden email]> wrote:
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.

that should read "The Pharo community is using the VM functionality to support Athens."  I have a bad habit of not looking up from the keyboard when I type :-(.  If there's a UX that sux it's autocorrect :-/

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 


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








--
_,,,^..^,,,_
best, Eliot


Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: The Inbox: EToys-cbc.293.mcz

Bert Freudenberg
In reply to this post by cbc
On Thu, Apr 20, 2017 at 11:29 PM, Chris Cunningham <[hidden email]> wrote:

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.

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 -



cbc
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: The Inbox: EToys-cbc.293.mcz

cbc


On Apr 21, 2017 11:23 AM, "Bert Freudenberg" <[hidden email]> wrote:
On Thu, Apr 20, 2017 at 11:29 PM, Chris Cunningham <[hidden email]> wrote:

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.

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. 

Thanks, 
cbc


Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: The Inbox: EToys-cbc.293.mcz

Bert Freudenberg
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:

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 -
 


cbc
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: The Inbox: EToys-cbc.293.mcz

cbc


On Mon, Apr 24, 2017 at 8:20 AM, Bert Freudenberg <[hidden email]> wrote:
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.

Wow.  I was writing this on my phone over the weekend, and didn't check what the phone had done to what I tried to write.  The entire previous paragraph was about code rot:
     Yes this approach can also ROT over time - but at least ExternalForm will be consistent with itself.  And it might not ROT as fast as leaving the mixed methods in there now. The only good way not to ROT is to put the classes back in, really.
I wasn't talking about speed at all.  Took me awhile to decode what I had intended to write - so I'm not surprised you came away with the angle you did.  Basically, there is reason to be concerned about packages diverging to the point where it isn't as easy as pulling it back in - especially if no one uses the code.

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:

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 -
 






Loading...