pygfx icon indicating copy to clipboard operation
pygfx copied to clipboard

Logic in materials is based on details about the wgpu renderer

Open almarklein opened this issue 2 years ago • 1 comments

While looking at #257 I ran into some abstraction leaks. We try to make the worldObject/geometry/material logic free from logic about the renderer. Even though we only have one real renderer (wgpu) this abstraction has helped us (I think) keep the code clean. But there are some things where it's off:

Last updated 18-04-2023

  • Some material properties are stored just as a property, while others are stored in the uniform buffer. Ideally this is a detail of the renderer.
  • Same for world object, though so far only the render_mask property.
  • Buffer and Texture set a few wgpu-specific private attributes. Theses are only used by the renderer, though a user could technically use _wgpu_usage when needed.
  • Actually, there are valid cases where a user should be able to set wgpu_usage, see #574.
  • Buffer and Texture accept a format that is generic, but also accept a wgpu.BufferFormat or wgpu.TextureFormat respectively.
  • The resource registry for buffers and textures uses wgpu-specific attributes of resources (writing this as I am working on #508, so maybe that has changed).
  • Material._wgpu_get_pick_info()
  • WorldObject._wgpu_get_pick_info()
  • utils/cube_camera.py -> uses/wraps a WgpuRenderer.
  • The show() util uses the WgpuRenderer, but fair enough since that's the default.

I'm not sure what best to do about this. Just accept it, we use the abstraction as a "rule of thumb" to keep on track? Or fix it, by moving the offending logic to the renderer?

almarklein avatar Mar 28 '22 21:03 almarklein

You're right, it's all supposed to be managed and tracked as part of the renderer. But we don't have to fix this right away.

Korijn avatar Mar 29 '22 07:03 Korijn

Decisions:

  • wgpu calls will be confined to renderers/backends.
  • render state may be managed on public API objects.
  • wgpu enums/flags may be used in public API objects. (e.g. culling mode)
  • We will embrace the fact that pygfx is based on the class of rendering backends that uses WGPU and similar implementations. As such "leaking abstractions" like properties that go into buffers or trigger shader compilations are OK to have in public API objects. It will be something that all foreseeable rendering backends require anyway.
  • As an example, the SVG renderer can still be implemented even if wgpu flags are used in public API objects, since wgpu can be imported without importing the rust binaries and without creating GPU devices/objects. We will not directly further invest in the SVG but it is free for community members to further develop it.

Actions:

  • Review the codebase to identify violations of these architectural constraints, and correct them
  • Review the codebase and address opportunities to simplify abstractions, where we are attempting to abstract away wgpu internal details, where it is actually only adding maintenance overhead and obfuscating underlying concepts

Korijn avatar Mar 06 '24 10:03 Korijn