The Trunk: Morphic-stlu.1682.mcz

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
2 messages Options
Reply | Threaded
Open this post in threaded view
|

The Trunk: Morphic-stlu.1682.mcz

commits-2
Marcel Taeumel uploaded a new version of Morphic to project The Trunk:
http://source.squeak.org/trunk/Morphic-stlu.1682.mcz

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

Name: Morphic-stlu.1682
Author: stlu
Time: 10 September 2020, 1:34:13.030274 pm
UUID: c81abdd7-0400-4fe2-bc43-61704bff7fe7
Ancestors: Morphic-mt.1679

Fixes FormCanvas's fast path for pure translations when using MatrixTransform2x3.

---

FormCanvas's current fast path for DisplayTransforms that are pure translations calls #offset, a method that is not (formally) part of the DisplayTransform protocol. While all transform types currently in trunk implement it, these implementations differ in their meaning:

        A) MatrixTransform2x3 >> #offset describes the translation applied to objects transformed by it on the grid
        B) MorphicTransform >> #offset describes the translation applied to the grid which objects transformed by it are placed on

All this ultimately amounts to is a simple negation. However, FormCanvas >> #transformBy:clippingTo:during:smoothing: assumes all display transforms to behave like MorphicTransform, causing MatrixTransform2x3 to translate in the exact opposite direction. Here an example:
       
        | formCanvas transform |
        transform := MatrixTransform2x3 withOffset: -100@ -100.
        "uncomment line below for not pure translation which should look (almost) identical"
        "transform := transform composedWithLocal: (MatrixTransform2x3 withScale: 0.99)."
        formCanvas := FormCanvas extent: 400@400.
        formCanvas fillColor: Color white.
        formCanvas
                translateBy: 200@200
                during: [:canvas |
                        canvas
                                transformBy: transform
                                clippingTo: (-1000@ -1000 corner: 1000@1000)
                                during: [:c | c fillRectangle: (0@0 extent: 100@100) color: Color red]].
        formCanvas form asMorph openInHand.

BUT since #offset has an entirely different meaning for the two transform types, it is IMO not a good idea to adapt either of its implementations to the other. I would not expect widely accepted matrix terminology to have a different meaning in Squeak. As a result, I currently only see 2 options:

        1. Introduce a new selector (e.g. #canvasOffset or #canvasTranslation)
        2. Use #localPointToGlobal:
       
I decided to go with the second option, since it reuses the existing interface and is hence automatically compatible with all existing display transforms. Also, while my imagination is failing to think of an example, assuming all transforms can answer to #offset might not be desirable? As for performance impact, the previous implementation is obviously faster, since it only required a quick method accessor. On my machine I benched the following:

        | transform |
        transform := MorphicTransform offset: 123@ -456.
        [transform offset] bench. " '152,000,000 per second. 6.59 nanoseconds per run. 0 % GC time.'"
        [transform localPointToGlobal: 0@0] bench. " '19,500,000 per second. 51.2 nanoseconds per run. 4.79808 % GC time.'"

I focused here on the MorphicTransform, since it is the only one actively used in Morphic (to my knowledge). To me this still seems like a rather tame slowdown, especially considering there only a couple hundred Morphs (at worst) calling #transformBy* in a typical Squeak image.

=============== Diff against Morphic-mt.1679 ===============

Item was changed:
  ----- Method: FormCanvas>>transformBy:clippingTo:during:smoothing: (in category 'drawing-support') -----
+ transformBy: aDisplayTransform clippingTo: aClipRect during: aBlock smoothing: cellSize
- transformBy: aDisplayTransform clippingTo: aClipRect during: aBlock smoothing: cellSize
 
  "Note: This method has been originally copied from TransformationMorph."
  | innerRect patchRect sourceQuad warp start subCanvas |
+ aDisplayTransform isPureTranslation ifTrue: [
+ ^ self
+ translateBy: (aDisplayTransform localPointToGlobal: 0@0) truncated
+ clippingTo: aClipRect
+ during: aBlock].
- (aDisplayTransform isPureTranslation) ifTrue:[
- ^aBlock value: (self copyOffset: aDisplayTransform offset negated truncated
- clipRect: aClipRect)
- ].
  "Prepare an appropriate warp from patch to innerRect"
  innerRect := aClipRect.
  patchRect := (aDisplayTransform globalBoundsToLocal: innerRect) truncated.
  sourceQuad := (aDisplayTransform sourceQuadFor: innerRect)
  collect: [:p | p - patchRect topLeft].
  warp := self warpFrom: sourceQuad toRect: innerRect.
  warp cellSize: cellSize.
 
  "Render the submorphs visible in the clipping rectangle, as patchForm"
  start := (self depth = 1 and: [self isShadowDrawing not])
  "If this is true B&W, then we need a first pass for erasure."
  ifTrue: [1] ifFalse: [2].
  start to: 2 do:
  [:i | "If i=1 we first make a shadow and erase it for opaque whites in B&W"
  subCanvas := self class extent: patchRect extent depth: self depth.
  i=1 ifTrue: [subCanvas shadowColor: Color black.
  warp combinationRule: Form erase]
  ifFalse: [self isShadowDrawing ifTrue:
  [subCanvas shadowColor: self shadowColor].
  warp combinationRule: (self depth = 32
  ifTrue: [Form blendAlphaScaled]
  ifFalse: [Form paint])].
  subCanvas
  translateBy: patchRect topLeft negated
  during: aBlock.
  warp sourceForm: subCanvas form; warpBits.
  warp sourceForm: nil.  subCanvas := nil "release space for next loop"]
  !


Reply | Threaded
Open this post in threaded view
|

Re: The Trunk: Morphic-stlu.1682.mcz

Eliot Miranda-2
Hi Marcel,

    maybe add commentary to MatrixTransform2x3 >>offset and MorphicTransform>>offset that these methods are short cuts and should not be used for transforming coordinates in Morphs, directing people to the real API.  And does some of the commit comment belong in a class comment and/or class side documentation method on transforms within Morphic?

_,,,^..^,,,_ (phone)

> On Oct 15, 2020, at 5:38 AM, [hidden email] wrote:
>
> ´╗┐Marcel Taeumel uploaded a new version of Morphic to project The Trunk:
> http://source.squeak.org/trunk/Morphic-stlu.1682.mcz
>
> ==================== Summary ====================
>
> Name: Morphic-stlu.1682
> Author: stlu
> Time: 10 September 2020, 1:34:13.030274 pm
> UUID: c81abdd7-0400-4fe2-bc43-61704bff7fe7
> Ancestors: Morphic-mt.1679
>
> Fixes FormCanvas's fast path for pure translations when using MatrixTransform2x3.
>
> ---
>
> FormCanvas's current fast path for DisplayTransforms that are pure translations calls #offset, a method that is not (formally) part of the DisplayTransform protocol. While all transform types currently in trunk implement it, these implementations differ in their meaning:
>
>    A) MatrixTransform2x3 >> #offset describes the translation applied to objects transformed by it on the grid
>    B) MorphicTransform >> #offset describes the translation applied to the grid which objects transformed by it are placed on
>
> All this ultimately amounts to is a simple negation. However, FormCanvas >> #transformBy:clippingTo:during:smoothing: assumes all display transforms to behave like MorphicTransform, causing MatrixTransform2x3 to translate in the exact opposite direction. Here an example:
>    
>    | formCanvas transform |
>    transform := MatrixTransform2x3 withOffset: -100@ -100.
>    "uncomment line below for not pure translation which should look (almost) identical"
>    "transform := transform composedWithLocal: (MatrixTransform2x3 withScale: 0.99)."
>    formCanvas := FormCanvas extent: 400@400.
>    formCanvas fillColor: Color white.
>    formCanvas
>        translateBy: 200@200
>        during: [:canvas |
>            canvas
>                transformBy: transform
>                clippingTo: (-1000@ -1000 corner: 1000@1000)
>                during: [:c | c fillRectangle: (0@0 extent: 100@100) color: Color red]].
>    formCanvas form asMorph openInHand.
>
> BUT since #offset has an entirely different meaning for the two transform types, it is IMO not a good idea to adapt either of its implementations to the other. I would not expect widely accepted matrix terminology to have a different meaning in Squeak. As a result, I currently only see 2 options:
>
>    1. Introduce a new selector (e.g. #canvasOffset or #canvasTranslation)
>    2. Use #localPointToGlobal:
>    
> I decided to go with the second option, since it reuses the existing interface and is hence automatically compatible with all existing display transforms. Also, while my imagination is failing to think of an example, assuming all transforms can answer to #offset might not be desirable? As for performance impact, the previous implementation is obviously faster, since it only required a quick method accessor. On my machine I benched the following:
>
>    | transform |
>    transform := MorphicTransform offset: 123@ -456.
>    [transform offset] bench. " '152,000,000 per second. 6.59 nanoseconds per run. 0 % GC time.'"
>    [transform localPointToGlobal: 0@0] bench. " '19,500,000 per second. 51.2 nanoseconds per run. 4.79808 % GC time.'"
>
> I focused here on the MorphicTransform, since it is the only one actively used in Morphic (to my knowledge). To me this still seems like a rather tame slowdown, especially considering there only a couple hundred Morphs (at worst) calling #transformBy* in a typical Squeak image.
>
> =============== Diff against Morphic-mt.1679 ===============
>
> Item was changed:
>  ----- Method: FormCanvas>>transformBy:clippingTo:during:smoothing: (in category 'drawing-support') -----
> + transformBy: aDisplayTransform clippingTo: aClipRect during: aBlock smoothing: cellSize
> - transformBy: aDisplayTransform clippingTo: aClipRect during: aBlock     smoothing: cellSize
>
>      "Note: This method has been originally copied from TransformationMorph."
>      | innerRect patchRect sourceQuad warp start subCanvas |
> +    aDisplayTransform isPureTranslation ifTrue: [
> +        ^ self
> +            translateBy: (aDisplayTransform localPointToGlobal: 0@0) truncated
> +            clippingTo: aClipRect
> +            during: aBlock].
> -    (aDisplayTransform isPureTranslation) ifTrue:[
> -        ^aBlock value: (self copyOffset: aDisplayTransform offset negated truncated
> -                            clipRect: aClipRect)
> -    ].
>      "Prepare an appropriate warp from patch to innerRect"
>      innerRect := aClipRect.
>      patchRect := (aDisplayTransform globalBoundsToLocal: innerRect) truncated.
>      sourceQuad := (aDisplayTransform sourceQuadFor: innerRect)
>                      collect: [:p | p - patchRect topLeft].
>      warp := self warpFrom: sourceQuad toRect: innerRect.
>      warp cellSize: cellSize.
>
>      "Render the submorphs visible in the clipping rectangle, as patchForm"
>      start := (self depth = 1 and: [self isShadowDrawing not])
>          "If this is true B&W, then we need a first pass for erasure."
>          ifTrue: [1] ifFalse: [2].
>      start to: 2 do:
>          [:i | "If i=1 we first make a shadow and erase it for opaque whites in B&W"
>          subCanvas := self class extent: patchRect extent depth: self depth.
>          i=1    ifTrue: [subCanvas shadowColor: Color black.
>                      warp combinationRule: Form erase]
>              ifFalse: [self isShadowDrawing ifTrue:
>                      [subCanvas shadowColor: self shadowColor].
>                  warp combinationRule: (self depth = 32
>                      ifTrue: [Form blendAlphaScaled]
>                      ifFalse: [Form paint])].
>          subCanvas
>              translateBy: patchRect topLeft negated
>              during: aBlock.
>          warp sourceForm: subCanvas form; warpBits.
>          warp sourceForm: nil.  subCanvas := nil "release space for next loop"]
>  !
>
>