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

Viewport transformation efficiency #98

Closed
white-axe opened this issue Feb 3, 2024 · 3 comments · Fixed by #132
Closed

Viewport transformation efficiency #98

white-axe opened this issue Feb 3, 2024 · 3 comments · Fixed by #132
Assignees

Comments

@white-axe
Copy link
Collaborator

Is your feature request related to a problem? Please describe.
Luminol's shaders have a 4D viewport transformation matrix. This is totally unnecessary because all we ever do to the viewport is translate and scale it.

Describe the solution you'd like
Replace the 4D viewport matrices with two vec2<f32>s: one that represents the position of the viewport and another that represents the scale along the X- and Y-axes.

Describe alternatives you've considered
If we need to do more than just translations and scaling to the viewport, we can use a 3D transformation matrix instead of a 4D one. A 3D transformation matrix is enough to encode any 2D affine transformation, a 4D transformation matrix is only required for 3D affine transformations.

@Speak2Erase Speak2Erase self-assigned this Feb 3, 2024
@Speak2Erase
Copy link
Member

I'm honestly not sure how to solve this one?

Viewports are currently projection matrices from model space (coordinates for vertices relative to each other) to clip space (position on the screen)

The way they're currently implemented, they actually don't do a lot for us, because egui calls RenderPass::set_viewport to set the region where custom paint callbacks are drawn on the screen.
This is both a curse and a blessing because it means that if we decide to draw things like events inside luminol_graphics::MapView we'll need to set the viewport ourselves, but it also means that we can effectively ditch Viewport (for events)

@white-axe
Copy link
Collaborator Author

We can still get rid of the projection matrix for Viewport though because, as far as I know, every single Viewport projection matrix we currently use is created using glam::Mat4::orthographic_rh, and that function just generates a matrix that represents some combination of translation and scaling (no rotations, shearing or other weird stuff). That can be reduced to just passing to the shader the translation as vec2<f32> and the scale as vec2<f32> instead of using a matrix.

@Speak2Erase
Copy link
Member

I think what I'll do is split viewport into the region on the screen being drawn to (shared between things drawn in the samw region), and any transformations being applied to what's being drawn

This is how 3d usually works (there's a transformation matrix that applies rotation, scaling, and translation to an object) and a projection matrix which applies perspective and normalizes coordinates to fit in clip space

@Speak2Erase Speak2Erase mentioned this issue Jun 15, 2024
5 tasks
@white-axe white-axe linked a pull request Jun 16, 2024 that will close this issue
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants