Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Default Canvas #376

Closed
FirefoxMetzger opened this issue Nov 5, 2022 · 19 comments
Closed

Default Canvas #376

FirefoxMetzger opened this issue Nov 5, 2022 · 19 comments

Comments

@FirefoxMetzger
Copy link
Contributor

In a previous discussion (#371 (comment)) I learned that you often don't need to interact with the canvas object directly when using pygfx. There are, of course, exceptions when this can be advantageous, but in general this is not the case. Because of this, we have introduced renderer.request_draw which wraps canvas.request_draw.

Taking this thought and thinking it to its conclusion I think it would make sense to also see if we can avoid all calls to canvas by default so that we only have to instantiate it when needed. In particular, I was wondering if we could change

from wgpu.gui.auto import WgpuCanvas, run
import pygfx as gfx

renderer = gfx.renderers.WgpuRenderer(WgpuCanvas())

to

import pygfx as gfx, run 

renderer = gfx.renderers.WgpuRenderer()

and make wgpu.gui.auto.WgpuCanvas the default canvas. We would, of course, keep the parameter and allow users to set it manually if needed. My angle here is to see if we can reduce boilerplate code.

On the same token, we could wrap run and avoid the import of wgpu for minimal examples. It's always nice to not have to import lower-level functionality when dealing with something on a more high level.

@Korijn
Copy link
Collaborator

Korijn commented Nov 6, 2022

I'm OK with this proposal. Seems like a good improvement to me.

How would you propose to wrap run?

@almarklein
Copy link
Collaborator

On the one hand, I'm reluctant, because I feel it makes the renderer and canvas feel intertwined. On the other hand I can see how this simplifies the API and lowers the barrier for newcomers. I've tried to think of variations but they ain't prettier. In the end I'm also in favor.

A gfx.run() could do something like:

def run():
    import wgpu 
    try:
        run = wgpu.gui.auto.run
    except AttributeError:
        ...
    run()

@Korijn
Copy link
Collaborator

Korijn commented Nov 6, 2022

I was thinking @FirefoxMetzger was probably expecting a method on the renderer... We can do both?

How about we provide gfx.canvas() to acquire a default canvas, gfx.run() to launch the event loop, and let the renderer call those functions in __init__ and renderer.run()? It's all just convenience layers so users can skim over details (until they need them in more complicated scenarios, of course).

@almarklein
Copy link
Collaborator

I would prefer creating only gfx.run(), since there is only one event loop. Putting it on the renderer might give the false impression that each renderer has its own loop or something.

@Korijn
Copy link
Collaborator

Korijn commented Nov 7, 2022

I'm OK with that. We just need to pick a position on the spectrum from "full truth" to "full simplicity". :)

So the plan is then to provide this API:

import pygfx as gfx

renderer = gfx.renderers.WgpuRenderer()

gfx.run()

And implement as follows (omitting a lot of unrelated details):

def canvas(*args, **kwargs):
    from wgpu.gui.auto import WgpuCanvas
    return WgpuCanvas(*args, **kwargs)


def run(*args, **kwargs):
    from wgpu.gui.auto import run
    run(*args, **kwargs)


class WgpuRenderer:
    def __init__(self, ..., canvas=None):
        if canvas is None:
            canvas = gfx.canvas()
        ...

I didn't really get the idea behind Almar's try/except construct.

@FirefoxMetzger
Copy link
Contributor Author

I was thinking @FirefoxMetzger was probably expecting a method on the renderer... We can do both?

Not per se. I was mainly thinking that it would be nice to not have to import wgpu while working with pygfx unless you have a specific need to customize something. So yes, something like gfx.run() seems intuitive.

@FirefoxMetzger
Copy link
Contributor Author

@almarklein Do we encounter the scenario that more than one backend is used at the same time? i.e., would we ever have to call wgpu.gui.qt.run AND wgpu.gui.qt.glfw.run within the same program?

@Korijn
Copy link
Collaborator

Korijn commented Nov 7, 2022

If users have such an advanced use case (which is technically possible), they would have to integrate the event loops, and one would become the controller of the other, and they would end up only calling run on the controlling event loop.

I guess it would be possible to call them both on different threads though, but I mean, these are really outlandish use cases.

@almarklein
Copy link
Collaborator

I agree with @Korijn. It's possible, but people doing that do/should use wgpu directly anyway.

@Korijn
Copy link
Collaborator

Korijn commented Nov 7, 2022

There's a couple more use cases for interacting with the canvas to consider:

  • examples/offscreen.py when doing offscreen rendering, you would retrieve the final image as a numpy array with img = canvas.draw()
  • examples/scene_subplots1.py when you have multiple viewports in a single canvas, you need to query the size with w, h = canvas.get_logical_size()
  • examples/text_with_qt.py I'm overlooking this one since it will be deprecated when proper text rendering support is merged

@FirefoxMetzger
Copy link
Contributor Author

I've been trying to come up with a good way to define gfx.run() and I don't think there is a smooth way to do so. The problem is that wgpu.gui.auto.run will only get us the correct event loop if we actually use the auto backend or explicitly use the backend that would be selected by the auto backend. This introduces a potential source of annoyance because something that is seemingly correct won't work:

import pygfx as gfx
import wgpu

renderer = gfx.renderers.WgpuRenderer(wgpu.gui.non-auto.WgpuCanvas())

#  starts the wrong event loop, because renderer doesn't use the `auto` canvas
gfx.run()

I have a few potential responses, but none of them area ideal:

  • We accept that gfx.run() only targets the auto backend and ask people to import run from wgpu otherwise
  • We introduce a renderer.run function that uses the renderer*s canvas to get the event loop
  • we introduce a gfx.gui_backend = wgpu.gui.auto singleton which users should use to control which canvas and run function gets used. Subsequently calling WgpuRenderer() will get a canvas from gfx.gui_backend and gfx.run will call the backend's run function.
  • we don't introduce gfx.run and just use a default canvas

None of these approaches are great. I think a gfx.gui_backend (with default value) comes closest to what we want, but prevents us from having more than one backend in the future.

I am starting to lean the opposite way and think that maybe it would be more consistent to deprecate renderer.request_draw and keep both the canvas and the renderer separate. What do you guys think? @Korijn @almarklein

examples/offscreen.py when doing offscreen rendering, you would retrieve the final image as a numpy array with img = canvas.draw()

I think this is a case where you'd want to interact with the canvas directly, so you would create it and pass it to the renderer explicitly.

examples/scene_subplots1.py when you have multiple viewports in a single canvas, you need to query the size with w, h = canvas.get_logical_size()

Same here. If you need to do things with the canvas you bring it in explicitly.

However, if you just want to create a scene and render it (like many other examples), there shouldn't be the need for you to drag along objects explicitly and my aim here is simply to reduce this boilerplate if possible.

@almarklein
Copy link
Collaborator

One option is to not import wgpu.auto in pygfx. Then in gfx.run() accessing wgpu.auto will raise an AttributeError when it's not being used by the user.

Technically, the user can still import the auto backend and still use another though ... It feels a bit messy indeed. Maybe this is a sign that we're trying to hide an abstraction that does not like to be hidden :)

@FirefoxMetzger
Copy link
Contributor Author

FirefoxMetzger commented Nov 7, 2022

Maybe this is a sign that we're trying to hide an abstraction that does not like to be hidden :)

I think so, yes; or at least we have the order wrong.

@almarklein I have a slightly off-topic rendering question again: Based on my current understanding, each world object in the scene comes with its own rendering pipeline. The pipeline might be re-used/shared, but it exists independently of the renderer. Did I get that right? If so, can I have multiple renderer objects draw the same scene to the same canvas?

@almarklein
Copy link
Collaborator

Each world object in the scene comes with its own rendering pipeline. The pipeline might be re-used/shared, but it exists independently of the renderer. Did I get that right?

Yes, world objects are not bound to a renderer.

can I have multiple renderer objects draw the same scene to the same canvas?

I think that it could, but you probably should not :) This is in part because the clear operation can only operate on the full screen, and also because the renderer has its own intermediate texture, so it would be a waste of memory usage.

That said, there are still some open ends. E.g. this means that one cannot have subplots that use different blending methods. Somewhat related is the wish of some users to access to the depth value. At some point we might need to revisit this.

@FirefoxMetzger
Copy link
Contributor Author

Hm, I see. In that case, we are indeed approaching this issue from the wrong angle and I am now fairly convinced that we should not add a default canvas inside the renderer.

I will need some time to come up with a more clear idea for an alternative. Currently, I'm thinking if it could make sense to add a new class (say Display) that could help streamline the process. Something like

import pygfx as gfx

scene = gfx.Scene()
... # populate the scene

display = gfx.default_display()
display.show(scene)  # calls the event loop

and if we want a custom animate:

import pygfx as gfx

scene = gfx.Scene()
... # populate the scene

display = gfx.default_display()

@display.draw_function
def animate():
    ...  # do awesome things

display.show(scene)

If we want a more functional style, we could also explore something like

import pygfx as gfx

def animate():
    ...  # awesome


(
    gfx.default_display()
    .add(
        Scene()
        .add(...)
    )
    .update(draw_function=animate)
    .run()
)

In any case, @Korijn @almarklein should we close this issue for now and make a new once once I have a more concrete suggestion?

@Korijn
Copy link
Collaborator

Korijn commented Nov 8, 2022

That's funny! Have a look at gfx.show, and the example.

@almarklein
Copy link
Collaborator

Taking a step back, the point of this exercise was to see if we can make most examples simpler by removing boilerplate code related to canvas. Your last suggestion indeed looks a lot like the gfx.show() util. Perhaps we should simply make more use of that in the examples (it was added after most examples were written)! 😄

Now that I think of it, the guide should probably start with an example that uses gfx.show() and only expand on the renderer et al. later!

I also like the idea of a custom animate function to use with gfx.show(), so you can e.g. rotate some cubes 😉 - but perhaps that would be more of a hook (and still call renderer.render() from gfx.show())

@FirefoxMetzger
Copy link
Contributor Author

That's funny! Have a look at gfx.show, and the example.

Nice! That goes very much in the direction I was thinking. We should definitely use it more; I'll update the examples as I port them into a gallery. I'm not 100% sure that having it as a function is the best approach (see below for my note on customization), but it definitely solves this issue :) I'll go ahead and close it

That said, like @almarklein suggested, it would be nice if show could use a custom draw function. Even better if we could customize other parts of the pipeline, too. For example, it would also be nice to choose a canvas other than the auto. This would be a different issue though.

@almarklein
Copy link
Collaborator

it would be nice if show could use a custom draw function

Could perhaps be an arg to show? But yeah, let's discuss elsewhere.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants