racket / pict

Other
15 stars 20 forks source link

Something is rotten in flip-x/flip-y #84

Open benknoble opened 3 months ago

benknoble commented 3 months ago

https://github.com/racket/pict/blob/8328b9049e6821f5f9b1ae089abf44bd6459f2fb/pict-lib/pict/private/utils.rkt#L1347

Introduced by me and @soegaard in https://github.com/racket/pict/commit/8328b9049e6821f5f9b1ae089abf44bd6459f2fb (cf. https://github.com/racket/pict/issues/29, https://github.com/racket/pict/pull/78).

I've observed this with a program like this one from my Frosthaven project, so you'll want to raco pkg install https://github.com/benknoble/frosthaven-manager to see the problem:

#lang racket

(require frosthaven-manager/gui/rich-text-display
         frosthaven-manager/curlique
         (only-in racket/gui/easy
                  render
                  window)
         pict)

(define (weird)
  (define boot-scale 3)
  (define (scale-point scale)
    {~> (-< car cdr) (>< (* scale)) cons})
  (ht-append
   -3
   (vr-append
    (hline (* boot-scale 8/3) 5)
    (hline (* boot-scale 8/4) 5)
    (hline (* boot-scale 8/3) 5))
   (rotate
    (flip-y
     (dc (λ (dc dx dy)
           (send dc draw-lines
                 (map (scale-point boot-scale)
                      '((0 . 0)
                        (2 . 0)
                        (3 . 1)
                        (4 . 0)
                        (6 . 0)
                        (8 . 1)
                        (4 . 4)
                        (4 . 10)
                        (0 . 10)
                        (0 . 0)))
                 dx dy))
         (* boot-scale 8)
         (* boot-scale 10)))
    (/ pi 6))))

(render
 (window
  (rich-text-display (list "first line" newline
                           "second" (weird) "line" newline
                           "third line"))))

In particular, make the window small enough that you need the vertical scroll, and watch what happens to the "boot" when you do. (Adding more lines gets more interesting.) Also note that in the final version where I fix the coordinates so I don't need flip-y, I use (rotate _ (/ pi -6)) instead of (rotate _ (/ pi 6)), which dovetails with comments below about rotations/etc.

@mflatt writes:

Just from a quick look, is there a reason to set the initial transformation matrix instead of using transform? I expected to see a combination of get-transformation, transform and set-transformation, instead. (If dc<%> were started from scratch, there would just be the current transformation — and a separate scale, offset, etc., would not be accessible.)

Another issue is that flip-x and flip-y lose track of nested picts. Impleemnting it with scale and inset, instead, would avoid those problems.

[RE: issue with (scale _ 1 -1)]: That's what inset can solve. But for rhombus/pict, https://github.com/racket/rhombus-prototype/blob/master/rhombus/pict/private/static.rhm#L156, I now remember that I went with hflip and then implemented vflip as hflip plus rotation, which may do better things with baselines.

A DC's state is four transformations, applied in order: initial matrix, translation, scale, and rotation. If the current transformation is based on non-identity variants of the last three, then directly setting the initial matrix one will have a weird effect, because the last three happen after that.

benknoble commented 3 months ago

Screenshots of various scroll positions:

Capture d’écran 2024-05-17 à 16 07 21 Capture d’écran 2024-05-17 à 16 07 26 Capture d’écran 2024-05-17 à 16 07 33