[bloc] feature envy -> move method close to data?
Locked
 
|
Hi Blockers
I do not really understand why drawOnAthensCanvas: is not defined on BlShape (especially when we see how many self space are defined in this method. And this is one of the few things I thought I understood from bloc (grouping the rendering in a separate object). drawOnAthensCanvas: aCanvas "Actually render receiver on aCanvas in local bounds. Override to customize. aCanvas is an instance of AthensCanvas aCanvas must not be nil" | pathTransform | pathTransform := aCanvas pathTransform. "First we fill" pathTransform restoreAfter: [ | path fillExtent fillTranslation | "We fill inside natural element's bounds" fillExtent := self shape path fillExtentFor: self shape inBounds: self localBounds. path := self shape path pathOn: aCanvas forExtent: fillExtent. "Translate fill if needed" fillTranslation := self shape path fillTranslationFor: self shape inBounds: self localBounds. pathTransform translateBy: fillTranslation. aCanvas setPaint: self shape fillPaint; drawShape: path ]. pathTransform restoreAfter: [ | path strokeExtent strokeTranslation | "We stroke inside natural element's bounds" strokeExtent := self shape path strokeExtentFor: self shape inBounds: self localBounds. path := self shape path pathOn: aCanvas forExtent: strokeExtent. "Translate stroke if needed" strokeTranslation := self shape path strokeTranslationFor: self shape inBounds: self localBounds. pathTransform translateBy: strokeTranslation. aCanvas setPaint: self shape strokePaint; drawShape: path ] |
Re: [bloc] feature envy -> move method close to data?
|
|
Hi,
Good question. The reason we want to have the morph be responsible because subclasses might decide to draw in a different way regardless of the shape being used. This gives us the maximum freedom for more specific blocs. Still, this is something to experiment with once we have more complicated examples built with Bloc. Cheers, Doru > On Feb 25, 2016, at 9:56 AM, stepharo <[hidden email]> wrote: > > Hi Blockers > > I do not really understand why drawOnAthensCanvas: is not defined on BlShape > (especially when we see how many self space are defined in this method. > And this is one of the few things I thought I understood from bloc (grouping the rendering in a separate object). > > > drawOnAthensCanvas: aCanvas > "Actually render receiver on aCanvas in local bounds. > Override to customize. > aCanvas is an instance of AthensCanvas > aCanvas must not be nil" > > | pathTransform | > pathTransform := aCanvas pathTransform. > "First we fill" > pathTransform > restoreAfter: > [ | path fillExtent fillTranslation | > "We fill inside natural element's bounds" > fillExtent := self shape path fillExtentFor: self shape inBounds: self localBounds. > path := self shape path pathOn: aCanvas forExtent: fillExtent. > "Translate fill if needed" > fillTranslation := self shape path > fillTranslationFor: self shape > inBounds: self localBounds. > pathTransform translateBy: fillTranslation. > aCanvas > setPaint: self shape fillPaint; > drawShape: path ]. > pathTransform > restoreAfter: > [ | path strokeExtent strokeTranslation | > "We stroke inside natural element's bounds" > strokeExtent := self shape path strokeExtentFor: self shape inBounds: self localBounds. > path := self shape path pathOn: aCanvas forExtent: strokeExtent. > "Translate stroke if needed" > strokeTranslation := self shape path > strokeTranslationFor: self shape > inBounds: self localBounds. > pathTransform translateBy: strokeTranslation. > aCanvas > setPaint: self shape strokePaint; > drawShape: path ] > -- www.tudorgirba.com www.feenk.com "Sometimes the best solution is not the best solution." |
Re: [bloc] feature envy -> move method close to data?
|
|
In reply to this post by stepharo
Hello,
very good remark. This is one of the main consequence of the design review we made recently. In the first design, this was the case, the drawing was delegated. For now, it is a known issue of the current design. Cheers Alain > On 25 févr. 2016, at 09:56, stepharo <[hidden email]> wrote: > > Hi Blockers > > I do not really understand why drawOnAthensCanvas: is not defined on BlShape > (especially when we see how many self space are defined in this method. > And this is one of the few things I thought I understood from bloc (grouping the rendering in a separate object). > > > drawOnAthensCanvas: aCanvas > "Actually render receiver on aCanvas in local bounds. > Override to customize. > aCanvas is an instance of AthensCanvas > aCanvas must not be nil" > > | pathTransform | > pathTransform := aCanvas pathTransform. > "First we fill" > pathTransform > restoreAfter: > [ | path fillExtent fillTranslation | > "We fill inside natural element's bounds" > fillExtent := self shape path fillExtentFor: self shape inBounds: self localBounds. > path := self shape path pathOn: aCanvas forExtent: fillExtent. > "Translate fill if needed" > fillTranslation := self shape path > fillTranslationFor: self shape > inBounds: self localBounds. > pathTransform translateBy: fillTranslation. > aCanvas > setPaint: self shape fillPaint; > drawShape: path ]. > pathTransform > restoreAfter: > [ | path strokeExtent strokeTranslation | > "We stroke inside natural element's bounds" > strokeExtent := self shape path strokeExtentFor: self shape inBounds: self localBounds. > path := self shape path pathOn: aCanvas forExtent: strokeExtent. > "Translate stroke if needed" > strokeTranslation := self shape path > strokeTranslationFor: self shape > inBounds: self localBounds. > pathTransform translateBy: strokeTranslation. > aCanvas > setPaint: self shape strokePaint; > drawShape: path ] > |
|
|
In reply to this post by Tudor Girba-2
To me this is pre optimisation
I do not see why we cannot do BlElement>>drawOnAthensCanvas: aCanvas self shape drawOnAthensCanvas: aCanvas with: self so far only localBounds from BlElement are used. And to me this is really strange to have an object responsible for the drawing not doing it. It blurs the responsibility. Stef Le 25/2/16 10:39, Tudor Girba a écrit : > Hi, > > Good question. > > The reason we want to have the morph be responsible because subclasses might decide to draw in a different way regardless of the shape being used. This gives us the maximum freedom for more specific blocs. Still, this is something to experiment with once we have more complicated examples built with Bloc. > > Cheers, > Doru > > >> On Feb 25, 2016, at 9:56 AM, stepharo <[hidden email]> wrote: >> >> Hi Blockers >> >> I do not really understand why drawOnAthensCanvas: is not defined on BlShape >> (especially when we see how many self space are defined in this method. >> And this is one of the few things I thought I understood from bloc (grouping the rendering in a separate object). >> >> >> drawOnAthensCanvas: aCanvas >> "Actually render receiver on aCanvas in local bounds. >> Override to customize. >> aCanvas is an instance of AthensCanvas >> aCanvas must not be nil" >> >> | pathTransform | >> pathTransform := aCanvas pathTransform. >> "First we fill" >> pathTransform >> restoreAfter: >> [ | path fillExtent fillTranslation | >> "We fill inside natural element's bounds" >> fillExtent := self shape path fillExtentFor: self shape inBounds: self localBounds. >> path := self shape path pathOn: aCanvas forExtent: fillExtent. >> "Translate fill if needed" >> fillTranslation := self shape path >> fillTranslationFor: self shape >> inBounds: self localBounds. >> pathTransform translateBy: fillTranslation. >> aCanvas >> setPaint: self shape fillPaint; >> drawShape: path ]. >> pathTransform >> restoreAfter: >> [ | path strokeExtent strokeTranslation | >> "We stroke inside natural element's bounds" >> strokeExtent := self shape path strokeExtentFor: self shape inBounds: self localBounds. >> path := self shape path pathOn: aCanvas forExtent: strokeExtent. >> "Translate stroke if needed" >> strokeTranslation := self shape path >> strokeTranslationFor: self shape >> inBounds: self localBounds. >> pathTransform translateBy: strokeTranslation. >> aCanvas >> setPaint: self shape strokePaint; >> drawShape: path ] >> > -- > www.tudorgirba.com > www.feenk.com > > "Sometimes the best solution is not the best solution." > > > |
Re: [bloc] feature envy -> move method close to data?
|
|
All arbitrary shapes are being drawing in the same way and it is enough to have only one method. Allowing shape to render itself would lead to misunderstanding: bloc user would have to look into two places (element and shape) to find out where drawing actually happens. On Feb 25, 2016 12:56 PM, "stepharo" <[hidden email]> wrote:
To me this is pre optimisation |
Re: [bloc] feature envy -> move method close to data?
|
|
In reply to this post by stepharo
> On 25 Feb 2016, at 12:51, stepharo <[hidden email]> wrote: > > To me this is pre optimisation > > I do not see why we cannot do > > BlElement>>drawOnAthensCanvas: aCanvas > > self shape drawOnAthensCanvas: aCanvas with: self > > so far only localBounds from BlElement are used. this was the previous implementation. and nothing prevent a particular BlElement subclass to redefine drawOnAthensCanvas: Alain > > Stef > |
|
|
In reply to this post by Aliaksei Syrel
Sorry but I do not buy your argument.
BlElement is in charge and it delegates it to BlShape. This is a quite common pattern and this is more logical that the object responsible for the shape drawing draw it in fact. It was like that in Bloc before so I do not see why this is not possible now. Le 25/2/16 13:37, Aliaksei Syrel a
écrit :
|
Re: [bloc] feature envy -> move method close to data?
|
|
Hello,
I think the problem is that, as far as we know now, it is not desirable that new subclasses of BlShape are created in case the drawing has to be redefined for a particular BlElements. For now, we flag it as a sensible point. We will fix or at least take a decision later. We need a layer as Brick on top of Bloc to see how it goes. Alain
|
Re: [bloc] feature envy -> move method close to data?
|
|
Exactly.
This is a sensible point. We need to learn a bit more from concrete implementations on top of Bloc before we decide. @Alex: Could you add a flag in the implementation? Doru p.s. I really love this kind of discussion > On Feb 26, 2016, at 9:33 AM, Alain Plantec via Pharo-dev <[hidden email]> wrote: > > > From: Alain Plantec <[hidden email]> > Subject: Re: [Pharo-dev] [bloc] feature envy -> move method close to data? > Date: February 26, 2016 at 9:32:16 AM GMT+1 > To: Pharo Development List <[hidden email]> > > > Hello, > I think the problem is that, as far as we know now, > it is not desirable that new subclasses of BlShape are created > in case the drawing has to be redefined for a particular BlElements. > For now, we flag it as a sensible point. > We will fix or at least take a decision later. > We need a layer as Brick on top of Bloc to see how it goes. > Alain > > > >> On 25 Feb 2016, at 23:01, stepharo <[hidden email]> wrote: >> >> Sorry but I do not buy your argument. >> BlElement is in charge and it delegates it to BlShape. >> This is a quite common pattern and this is more logical that the object responsible for the shape drawing draw it in fact. >> It was like that in Bloc before so I do not see why this is not possible now. >> >> Le 25/2/16 13:37, Aliaksei Syrel a écrit : >>> All arbitrary shapes are being drawing in the same way and it is enough to have only one method. Allowing shape to render itself would lead to misunderstanding: bloc user would have to look into two places (element and shape) to find out where drawing actually happens. >>> >>> On Feb 25, 2016 12:56 PM, "stepharo" <[hidden email]> wrote: >>> To me this is pre optimisation >>> >>> I do not see why we cannot do >>> >>> BlElement>>drawOnAthensCanvas: aCanvas >>> >>> self shape drawOnAthensCanvas: aCanvas with: self >>> >>> so far only localBounds from BlElement are used. >>> >>> And to me this is really strange to have an object responsible for the drawing not doing it. >>> It blurs the responsibility. >>> >>> Stef >>> >>> Le 25/2/16 10:39, Tudor Girba a écrit : >>> Hi, >>> >>> Good question. >>> >>> The reason we want to have the morph be responsible because subclasses might decide to draw in a different way regardless of the shape being used. This gives us the maximum freedom for more specific blocs. Still, this is something to experiment with once we have more complicated examples built with Bloc. >>> >>> Cheers, >>> Doru >>> >>> >>> On Feb 25, 2016, at 9:56 AM, stepharo <[hidden email]> wrote: >>> >>> Hi Blockers >>> >>> I do not really understand why drawOnAthensCanvas: is not defined on BlShape >>> (especially when we see how many self space are defined in this method. >>> And this is one of the few things I thought I understood from bloc (grouping the rendering in a separate object). >>> >>> >>> drawOnAthensCanvas: aCanvas >>> "Actually render receiver on aCanvas in local bounds. >>> Override to customize. >>> aCanvas is an instance of AthensCanvas >>> aCanvas must not be nil" >>> >>> | pathTransform | >>> pathTransform := aCanvas pathTransform. >>> "First we fill" >>> pathTransform >>> restoreAfter: >>> [ | path fillExtent fillTranslation | >>> "We fill inside natural element's bounds" >>> fillExtent := self shape path fillExtentFor: self shape inBounds: self localBounds. >>> path := self shape path pathOn: aCanvas forExtent: fillExtent. >>> "Translate fill if needed" >>> fillTranslation := self shape path >>> fillTranslationFor: self shape >>> inBounds: self localBounds. >>> pathTransform translateBy: fillTranslation. >>> aCanvas >>> setPaint: self shape fillPaint; >>> drawShape: path ]. >>> pathTransform >>> restoreAfter: >>> [ | path strokeExtent strokeTranslation | >>> "We stroke inside natural element's bounds" >>> strokeExtent := self shape path strokeExtentFor: self shape inBounds: self localBounds. >>> path := self shape path pathOn: aCanvas forExtent: strokeExtent. >>> "Translate stroke if needed" >>> strokeTranslation := self shape path >>> strokeTranslationFor: self shape >>> inBounds: self localBounds. >>> pathTransform translateBy: strokeTranslation. >>> aCanvas >>> setPaint: self shape strokePaint; >>> drawShape: path ] >>> >>> -- >>> www.tudorgirba.com >>> www.feenk.com >>> >>> "Sometimes the best solution is not the best solution." >>> >>> >>> >>> >>> >> > > > -- www.tudorgirba.com www.feenk.com "We can create beautiful models in a vacuum. But, to get them effective we have to deal with the inconvenience of reality." |
Re: [bloc] feature envy -> move method close to data?
|
|
In reply to this post by Pharo Smalltalk Developers mailing list
On 26-02-16 09:33, Alain Plantec via Pharo-dev wrote:
>I think the problem is that, as far as we know now, >it is not desirable that new subclasses of BlShape are created >in case the drawing has to be redefined for a particular BlElements. Can you give an example of this? Stephan |
|
|
In reply to this post by Tudor Girba-2
I will move the method and add a delegation
and it should work for handMorph too. <hypothesis> Stefan a case is that we may have a BlElement whose drawing is done by the hardware <hypothesis> Le 26/2/16 10:14, Tudor Girba a écrit : > Exactly. > > This is a sensible point. We need to learn a bit more from concrete implementations on top of Bloc before we decide. > > @Alex: Could you add a flag in the implementation? > > Doru > > p.s. I really love this kind of discussion OK we will continue to have them. I hope to get some time to start to write some tests based on the method comments in BlElement and enrich the class comments as well. Stef > > >> On Feb 26, 2016, at 9:33 AM, Alain Plantec via Pharo-dev <[hidden email]> wrote: >> >> >> From: Alain Plantec <[hidden email]> >> Subject: Re: [Pharo-dev] [bloc] feature envy -> move method close to data? >> Date: February 26, 2016 at 9:32:16 AM GMT+1 >> To: Pharo Development List <[hidden email]> >> >> >> Hello, >> I think the problem is that, as far as we know now, >> it is not desirable that new subclasses of BlShape are created >> in case the drawing has to be redefined for a particular BlElements. >> For now, we flag it as a sensible point. >> We will fix or at least take a decision later. >> We need a layer as Brick on top of Bloc to see how it goes. >> Alain >> >> >> >>> On 25 Feb 2016, at 23:01, stepharo <[hidden email]> wrote: >>> >>> Sorry but I do not buy your argument. >>> BlElement is in charge and it delegates it to BlShape. >>> This is a quite common pattern and this is more logical that the object responsible for the shape drawing draw it in fact. >>> It was like that in Bloc before so I do not see why this is not possible now. >>> >>> Le 25/2/16 13:37, Aliaksei Syrel a écrit : >>>> All arbitrary shapes are being drawing in the same way and it is enough to have only one method. Allowing shape to render itself would lead to misunderstanding: bloc user would have to look into two places (element and shape) to find out where drawing actually happens. >>>> >>>> On Feb 25, 2016 12:56 PM, "stepharo" <[hidden email]> wrote: >>>> To me this is pre optimisation >>>> >>>> I do not see why we cannot do >>>> >>>> BlElement>>drawOnAthensCanvas: aCanvas >>>> >>>> self shape drawOnAthensCanvas: aCanvas with: self >>>> >>>> so far only localBounds from BlElement are used. >>>> >>>> And to me this is really strange to have an object responsible for the drawing not doing it. >>>> It blurs the responsibility. >>>> >>>> Stef >>>> >>>> Le 25/2/16 10:39, Tudor Girba a écrit : >>>> Hi, >>>> >>>> Good question. >>>> >>>> The reason we want to have the morph be responsible because subclasses might decide to draw in a different way regardless of the shape being used. This gives us the maximum freedom for more specific blocs. Still, this is something to experiment with once we have more complicated examples built with Bloc. >>>> >>>> Cheers, >>>> Doru >>>> >>>> >>>> On Feb 25, 2016, at 9:56 AM, stepharo <[hidden email]> wrote: >>>> >>>> Hi Blockers >>>> >>>> I do not really understand why drawOnAthensCanvas: is not defined on BlShape >>>> (especially when we see how many self space are defined in this method. >>>> And this is one of the few things I thought I understood from bloc (grouping the rendering in a separate object). >>>> >>>> >>>> drawOnAthensCanvas: aCanvas >>>> "Actually render receiver on aCanvas in local bounds. >>>> Override to customize. >>>> aCanvas is an instance of AthensCanvas >>>> aCanvas must not be nil" >>>> >>>> | pathTransform | >>>> pathTransform := aCanvas pathTransform. >>>> "First we fill" >>>> pathTransform >>>> restoreAfter: >>>> [ | path fillExtent fillTranslation | >>>> "We fill inside natural element's bounds" >>>> fillExtent := self shape path fillExtentFor: self shape inBounds: self localBounds. >>>> path := self shape path pathOn: aCanvas forExtent: fillExtent. >>>> "Translate fill if needed" >>>> fillTranslation := self shape path >>>> fillTranslationFor: self shape >>>> inBounds: self localBounds. >>>> pathTransform translateBy: fillTranslation. >>>> aCanvas >>>> setPaint: self shape fillPaint; >>>> drawShape: path ]. >>>> pathTransform >>>> restoreAfter: >>>> [ | path strokeExtent strokeTranslation | >>>> "We stroke inside natural element's bounds" >>>> strokeExtent := self shape path strokeExtentFor: self shape inBounds: self localBounds. >>>> path := self shape path pathOn: aCanvas forExtent: strokeExtent. >>>> "Translate stroke if needed" >>>> strokeTranslation := self shape path >>>> strokeTranslationFor: self shape >>>> inBounds: self localBounds. >>>> pathTransform translateBy: strokeTranslation. >>>> aCanvas >>>> setPaint: self shape strokePaint; >>>> drawShape: path ] >>>> >>>> -- >>>> www.tudorgirba.com >>>> www.feenk.com >>>> >>>> "Sometimes the best solution is not the best solution." >>>> >>>> >>>> >>>> >>>> >> >> > -- > www.tudorgirba.com > www.feenk.com > > "We can create beautiful models in a vacuum. > But, to get them effective we have to deal with the inconvenience of reality." > > > |
«
Return to Pharo Smalltalk Developers
|
1 view|%1 views
| Free forum by Nabble | Edit this page |