ScenicFramework / scenic

Core Scenic library
Apache License 2.0
1.99k stars 137 forks source link

rapid redraws of the graph have tearing #262

Closed bcardarella closed 11 months ago

bcardarella commented 2 years ago

My animation library will draw to the screen ever 16ms for each animation event. See the attached video. The white arrow demonstrates the tearing most easily.

https://user-images.githubusercontent.com/18524/159074527-4a95570c-07dc-4fa7-80ea-11edee3e764f.mov

Here is a zoom of the tear happening:

Screen Shot 2022-03-18 at 3 54 42 PM

It's not clear to me why this is happening.

I don't even think it is the animation library. This next video demonstrates the arrow being drawn directly to a position without any animation:

https://user-images.githubusercontent.com/18524/159074989-2b6d231d-51fe-42f5-b074-0a69086d8b6c.mov

And here is a zoom of the tearing:

Screen Shot 2022-03-18 at 3 57 42 PM

I'm calling it tearing, no idea what else to call it.

Here is the definition of this arrow:

def arrow(graph, degrees, radius, opts) do
  path(graph, [:begin, {:move_to, -10, -15}, {:line_to, 0, 15}, {:line_to, 10, -15}, {:line_to, 0, -10}, {:line_to, -10, -15}, :close_path], Keyword.merge(opts, radius: radius))
  |> Graph.modify(opts[:id], &move_arrow(&1, degrees))
end

defp move_arrow(arrow, degrees) do
  opts = Map.get(arrow, :opts, %{})
  radius = Keyword.get(opts, :radius, 0)
  radians = Math.degrees_to_radians(degrees)
  degrees = Math.rotate_degrees_for_compass(degrees)

  Scenic.Primitive.merge_opts(arrow, translate: Arc.calculate_point({0, 0}, degrees, radius), rotate: radians)
end
crertel commented 2 years ago

Have you tried this on a machine with a different GPU?

bcardarella commented 2 years ago

Is Scenic using the GPU?

boydm commented 2 years ago

Yes. Scenic uses the GPU. There could be a backend that doesn't, but I haven't written that yet.

Brian, could you please post in the part of your config.exs that sets up the viewport. In particular, I'm interested in the config for the local_driver.

boydm commented 2 years ago

Also, what machine is this running on?

bcardarella commented 2 years ago
Screen Shot 2022-03-21 at 6 04 48 PM
bcardarella commented 2 years ago

Config:

config :weather_station, :viewport,
  name: :main_viewport,
  size: {800, 600},
  theme: :dark,
  default_scene: WeatherStation.Scene.Home,
  drivers: [
    [
      module: Scenic.Driver.Local,
      name: :local,
      window: [resizeable: false, title: "weather_station"],
      on_close: :stop_system
    ]
  ]
boydm commented 2 years ago

Thanks. This is not an issue with scenic itself, but might be an issue with scenic_driver_local. Possibly an issue with how it uses the Intel UHD Graphics card.

The @opts_scema section of https://github.com/ScenicFramework/scenic_driver_local/blob/main/lib/driver.ex show the tweekable config parameters for the local driver.

In this case, we want to try adjusting :limit_ms. This limits how quickly the screen actually redraw. The default is 29 ms, which is chosen to be slightly shorter than the 30ms refresh rate of many monitors.

You are sending data to it at roughly twice the frequency of the refresh rate. In theory, this should discard half of your updates. What's interesting is if the driver receives an update at just the right time while it is doing a draw, does that interfere with the currently drawing frame. If yes, there might be a lock/coordination issue that can be fixed in the driver.

Part of the design is that the updates shouldn't even get sent down to the renderer if they are coming too fast. This might be showing a bug.

So... I'd like you try overriding the driver's refresh rate. If you have a performance counter you can keep open that shows the % use of the video card, that would be excellent.

Try setting limit_ms: 8 limit_ms: 16 limit_ms: 60

That config goes in the Local section...

  drivers: [
    [
      limit_ms: 8,
      module: Scenic.Driver.Local,
      name: :local,
      window: [resizeable: false, title: "weather_station"],
      on_close: :stop_system
    ]

8ms is faster than your update speed, but might not allow enough time for the actual render to complete. I'd like to know if this improves, or worsens the "tearing".

16ms should be pretty close to your update rate.

60ms is very clearly longer than your update rate. most updates will be ignored and it should only take the latest every 60ms or so. I'd like to know if you still see tearing.

boydm commented 2 years ago

There are other driver opts you can play with.

For a good time, try...

  drivers: [
    [
      position: [scaled: true, centered: true, orientation: :upside_down],
      module: Scenic.Driver.Local,
      name: :local,
      window: [resizeable: false, title: "weather_station"],
      on_close: :stop_system
    ]
crertel commented 2 years ago

@boydm how would we feel about maybe having Scenic do v-sync?

boydm commented 2 years ago

That would be specific to the local driver. Scenic itself shouldn't know about that, but the driver can.

bcardarella commented 2 years ago

You are sending data to it at roughly twice the frequency of the refresh rate. In theory, this should discard half of your updates

I have just started to think about debouncing the animation frames. I should be able to debounce within limit_ms. Right now I am aiming at 60fps which needs 16ms updates but from what you're saying I should be aiming at 30fps?

bcardarella commented 2 years ago

Although, I may have to rethink some of how I implemented. Right now animation frames are pushed to the process' mailbox and are independent but tied to values that are changing over time. So there could be N rendering that happen within the current 16ms window. I felt like this may be problematic and I may need to think about how to better manage this

bcardarella commented 2 years ago

I just did a test and removed the animations out the equation and am still getting this rendering issue:

Screen Shot 2022-03-21 at 8 16 36 PM
bcardarella commented 2 years ago

This was still at the default limit_ms but I figured without the animations it didn't matter

boydm commented 2 years ago

That is super weird.

Can I see the bit of graph or whatever that draws the arrow?

boydm commented 2 years ago

The goal of the limit_msis to do the de-bouncing at the driver level. Different drivers have different refresh rates and it should be transparent to the scenes above them.

Having said that, it's still a good idea to roughly sync up the update rate with the draw rate just to use less power on the device.

bcardarella commented 2 years ago
def arrow(graph, nil, radius, opts), do: arrow(graph, 0, radius, opts)
def arrow(graph, degrees, radius, opts) do
  path(graph, [:begin, {:move_to, -10, -15}, {:line_to, 0, 15}, {:line_to, 10, -15}, {:line_to, 0, -10}, {:line_to, -10, -15}, :close_path], Keyword.merge(opts, radius: radius))
  |> Graph.modify(opts[:id], &move_arrow(&1, degrees))
end

# This function will move the arrow to the proper position on the circumfrence of a circle
# as well as rotate the arrow head to always be pointing to the origin of that circle
defp move_arrow(arrow, degrees) do
  opts = Map.get(arrow, :opts, %{})
  radius = Keyword.get(opts, :radius, 0)

  # Math is my own module
  radians = Math.degrees_to_radians(degrees)
  degrees = Math.rotate_degrees_for_compass(degrees)

  Scenic.Primitive.merge_opts(arrow, translate: Arc.calculate_point({0, 0}, degrees, radius), rotate: radians)
end
crertel commented 2 years ago

Could you try replacing your path() invocation with:

path(graph, [:begin, 
  {:move_to, -10, -15}, 
  {:line_to, 0, -10}, 
  {:line_to, 10, -15},
  {:line_to, 0, 15},   
  {:line_to, -10, -15}, 
  :close_path], Keyword.merge(opts, radius: radius))

and letting us know if that maybe helps at all?

crertel commented 2 years ago

And if that doesn't work, could we try just a triangle by itself:

path(graph, [:begin, 
  {:move_to, -10, -15}, 
  {:line_to, 0, 15},   
  {:line_to, 10, -15},
  {:line_to, -10, -15}, 
  :close_path], Keyword.merge(opts, radius: radius))
boydm commented 2 years ago

I built a very simple scene that just draws the triangle and rotates it quickly. I don't see any visual corruption.

Brian - could you please build/run just this scene and let us know if you see visual artifacts?

defmodule Example.Scene.Spinner do
  use Scenic.Scene
  alias Scenic.Graph
  import Scenic.Primitives

  @graph Graph.build()
    |> path(
      [
        :begin,
        {:move_to, -10, -15},
        {:line_to, 0, 15},
        {:line_to, 10, -15},
        {:line_to, 0, -10},
        {:line_to, -10, -15},
        :close_path
      ],
      stroke: {4, :green},
      translate: {200, 200},
      id: :arrow
    )

  @doc false
  @impl Scenic.Scene
  def init(scene, _param, _opts) do
    # Align the buttons and the temperature display. This has to be done at runtime
    # as we won't know the viewport dimensions until then.
    {width, _} = scene.viewport.size
    col = width / 6

    {:ok, timer} = :timer.send_interval(8, :turn)

    scene =
      scene
      |> assign( angle: 0, timer: timer )
      |> push_graph( @graph )

    {:ok, scene}
  end

  def handle_info( :turn, %{assigns: %{angle: angle}} = scene ) do
    angle = angle + 0.01

    scene =
      scene
      |> assign( :angle, angle )
      |> push_graph( Graph.modify(@graph, :arrow, &update_opts(&1, rotate: angle)) )

    {:noreply, scene}
  end

end
boydm commented 2 years ago

Here is a script based version of the same thing. This uses a static script to draw the arrow. This script is pushed to the viewport only once when the scene is started. The updates on the timer then only update the transforms on the reference to the script.

This is much more efficient than sending the otherwise static arrow every time.

defmodule Example.Scene.SpinScript do
  use Scenic.Scene
  alias Scenic.Graph
  alias Scenic.Script
  import Scenic.Primitives

  @script Script.start()
      |> Script.push_state()
      |> Script.stroke_width(4)
      |> Script.stroke_color(:green)
      |> Script.begin_path()
      |> Script.move_to(-10, -15)
      |> Script.line_to(0, 15)
      |> Script.line_to(10, -15)
      |> Script.line_to(0, -10)
      |> Script.line_to(-10, -15)
      |> Script.close_path()
      |> Script.stroke_path()
      |> Script.pop_state()
      |> Script.finish()

  @graph Graph.build()
      |> script( "arrow_script", id: :arrow, translate: {400, 300}, scale: 3 )

  def init(scene, _param, _opts) do
    # Align the buttons and the temperature display. This has to be done at runtime
    # as we won't know the viewport dimensions until then.
    {width, _} = scene.viewport.size
    col = width / 6

    {:ok, timer} = :timer.send_interval(8, :turn)

    scene =
      scene
      |> assign( angle: 0, timer: timer )
      |> push_script( @script, "arrow_script" )
      |> push_graph( @graph )

    {:ok, scene}
  end

  def handle_info( :turn, %{assigns: %{angle: angle}} = scene ) do
    angle = angle + 0.01

    scene =
      scene
      |> assign( :angle, angle )
      |> push_graph( Graph.modify(@graph, :arrow, &update_opts(&1, rotate: angle)) )

    {:noreply, scene}
  end

end

This also has no tearing or other visual corruption on my machine. I'd like to know how it looks on yours.

crertel commented 11 months ago

Closing out for now, let us know if you're able to answer some of the debugging questions upthread.

bcardarella commented 11 months ago

@crertel as much as I'd like to help this was opened a long time ago and I've not touched that project in quite some time, I'm completely out of context on this now.

crertel commented 11 months ago

No worries, thank you!