vydd / sketch

A Common Lisp framework for the creation of electronic art, visual design, game prototyping, game making, computer graphics, exploration of human-computer interaction, and more.
MIT License
1.4k stars 67 forks source link

copy-pixels overrides background colour depending on window size #69

Closed Kevinpgalligan closed 7 months ago

Kevinpgalligan commented 1 year ago

This works, draws random black circles on a white background:

 (defsketch bug
          ((width 800)
           (height 600)
           (copy-pixels t)
           (pen (make-pen :fill +black+)))
        (with-pen pen
          (circle (random width) (random height) (random 10))))
  (defmethod setup ((instance bug) &key &allow-other-keys)
              (background +white+))

But if you change both the width & height to 400, for example, then the background goes black. It also seems like an initial window comes up, then gets closed and a new one opens up with the desired dimensions. I'm on Linux and have Intel graphics hardware, if that makes a difference.

edit: My guess is that sketch restarts the window in order to initialise it with the proper dimensions, but then doesn't call setup afterwards. I'm not sure why this behaviour is dependent on the width and height, though.

Gleefre commented 1 year ago

It seems that this behaviour depends on the width and height because the window is created with the default size of (800 600) which is inherited from kit.sdl2.

Based on this, there is an easy workaround: if you specify initial width and height with :w and :h (parameters for kit.sdl2 window), the background does not goes black.

(make-instance 'bug :width 400 :w 400 :height 400 :h 400) ; background remains white
swapneils commented 1 year ago

I'm encountering a similar issue with the Brownian example code (on Windows), though I'm not sure whether it's the same; a window pops up and immediately disappears, followed by another one of the specified size.

However, instead of just being set to black, the second window's background keeps flickering between black and an incorrect representation of the specified color (e.g. (background (rgb 255 0 5)) in the "setup" method makes a flicker between pure purple (rgb 255 0 255) and black). This is also the case with your "bug" code above. Incidentally, in the Brownian example the stroke also alternates between white and the specified gray color, although bug doesn't seem to exhibit that problem.

Running without copy-pixels in the sketch definition works fine with no flickering.

Matching the expected dimensions (or changing the :w and :h parameters by copying Kevin's and Gleefre's code above) removes (or perhaps hides?) the disappearing initial window but doesn't fix the problem with the flickering background.

UPDATE:

On further investigation, it appears that the flickering is actually because 2 separate canvases are alternating between each other, which doesn't seem to be the same as this issue. Moved to #75

Kevinpgalligan commented 1 year ago

This is manifesting in one of my sketches where I gradually fill in the window with rects. The first few rects disappear, presumably because the window is re-created, unless I follow @Gleefre's suggestion of passing the :w and :h parameters. Even when I pass :w and :h, the last few rects seem to flicker between two colours, which may be similar to what you're reporting, @swapneils.

Kevinpgalligan commented 8 months ago

Recording some of the debugging I've done for this issue.

Here's a minimal example to reproduce the bug. It's supposed to draw a red circle on the first render. Instead, a green screen appears for a bit (part of sketch's setup, see initialize-gl). The red circle gets drawn on the green background. Then it seems that setup gets called, overwriting the initial render. So it seems that there's a rogue render call before the end of the sketch initialization / setup.

(defsketch resizebug
    ((width 400)
     (height 400)
     (copy-pixels t)
     (first-render t))
  (with-pen (make-pen :fill
                      (if first-render
                          (progn (setf first-render nil)
                                 +red+)
                          +white+))
    (circle (random width) (random height) 30))
  (sleep 1))

(defmethod setup ((instance resizebug) &key &allow-other-keys)
  (background +black+)
  (sleep 1)

I originally thought this was due to the window being resized, and that there were renders happening before the screen resize that got discarded due to the resize. Here's the order of events with the screen size at each stage.

Window event 800 600
RENDER 800 600
Window event 400 400
RENDER 400 400
RENDER 400 400

But even setting the width & height to 800/600, the sdl2kit default, removes the 2nd window event but doesn't fix the bug.

Window event 800 600
RENDER 800 600
RENDER 800 600

Bug was reproduced on @Gleefre's system as well.

Kevinpgalligan commented 8 months ago

I think I've found one of the root causes for this.

In the render function, the variable %restart is used to check whether the sketch is starting/restarting. It's initialised to 2. render calls setup when that flag is reduced to 0. Each render call reduces it by 1. So that's why it's not calling setup until the 2nd render. With (copy-pixels t), then, the first render is being overwritten by setup.

%restart used to be a boolean flag, it was changed to a counter here: https://github.com/vydd/sketch/commit/d0181a3142117086a68b8ba463f92a696312ab9e

Since this bug predates that commit, there may be another root cause. In my version of sketch, I've removed the call to render in the window-event method - that may be another cause.

And, in fact, in some of my testing I've seen multiple render calls happening before the 2nd resize event, so there may be multiple rogue renders slipping in before the setup/initialisation is finished. I will do some testing and see if I can come up with a fix.

Kevinpgalligan commented 8 months ago

Actually, this is different from the original problem I reported when I created this issue. Running the bug sketch above now gives a white background with black circles, even if width and height are changed to 400/400. So the original problem has been solved, but the behaviour with (copy-pixels t) is still buggy.

Kevinpgalligan commented 8 months ago

I removed the render call from the :size-changed event and reverted %restart to being a boolean flag.

Result: resizebug draws the red circle on a black background instead of a green background, but then the red circle disappears. So we're not rendering prematurely, i.e. before setup, but somehow the first render is still getting discarded.

I printed the stack trace from each call to render, it seems that 2 renders are coming from :exposed window events: https://gist.github.com/Kevinpgalligan/d4537edd9649bccff4880a02e49abc51

@Gleefre found that this is due to default behaviour in sdl2kit: https://github.com/lispgames/sdl2kit/blob/master/src/window.lisp#L170-L171

I confirmed that setup is indeed being called during the first call to render.

I overwrote the default behaviour for :exposed window events so that there's no call to render:

(defmethod kit.sdl2:window-event :around ((window sketch) (type (eql :exposed)) timestamp data1 data2)
  ;; This overwrites the default behaviour of sdl2kit, which triggers a render that we don't want.
  )

Now the red circle appears on a black background after the 1st render, then after the 2nd frame it has been erased and it starts drawing white circles. So, the first render is still being discarded somehow. The root cause is still unknown.

Kevinpgalligan commented 8 months ago

The first render is not discarded when we set the dimensions of the window to 800x600 (i.e. the red circle doesn't disappear).

So, I think this bug is due to

  1. setup being called after the first render (easily fixed).
  2. Window resizing.

To fix (2), we could condition the rendering on whether the window has reached its final size. That seems hacky, though, so I would prefer to avoid the resize in the first place, which requires evaluating width and height and passing them as :w and :h to the constructor of sdl2's window class. See @Gleefre thoughts on that here: https://github.com/vydd/sketch/pull/71#issuecomment-1615995049

Kevinpgalligan commented 7 months ago

I'm still working on a fix for (2), see: https://github.com/Kevinpgalligan/sketch/tree/separate-sketch-window-class

The idea is to create a separate sketch-window class so that we can initialize all the slots of the sketch class before creating the window. It's turning out to be surprisingly tricky, though -- e.g. there may be a circular dependency between the slots and the window.

I will also submit a fix for (1).

Kevinpgalligan commented 7 months ago

I've confirmed that this is fixed by the latest commits -- the resizebug sketch I shared above now works as expected. There's still a bug with copy-pixels t related to double-buffering, but that can be tracked in this separate issue: https://github.com/vydd/sketch/issues/75