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

Fix render matrix for VelloScenes in Bevy UI nodes #59

Merged
merged 6 commits into from
Jun 12, 2024

Conversation

dannymcgee
Copy link
Contributor

This PR fixes a couple issues I had when trying to use a VelloScene with Bevy's UI:

  1. The Y-axis was flipped, so that setting any top or bottom value on the Style component caused the VelloScene to move in the opposite direction from the node
  2. Bevy's layout system puts the transform origin at the center of the node's computed bounding box, which is awkward for a canvas-like API where you expect to be issuing draw commands with the origin at the bounding box corner.

That said, I know there are different conventions for whether the Y-origin is top or bottom, and I'm honestly not sure where Vello's origin is under normal circumstances. Let me know if you'd like to preserve a bottom-left origin for Vello drawing commands while still positioning the Vello scene so that it aligns with the Bevy node — it shouldn't be overly difficult to make that adjustment.

I haven't tested this super exhaustively, but I did verify that the original "scene" example still works as expected, and added a new "scene-ui" example for testing the changes made here. Let me know if you have any questions, concerns, or would like to see any changes!

@simbleau
Copy link
Member

simbleau commented Jun 1, 2024

Thank you for the PR! Please update the CHANGELOG as well with the change.

@seabassjh please review

@seabassjh seabassjh enabled auto-merge June 12, 2024 17:57
@seabassjh seabassjh added this pull request to the merge queue Jun 12, 2024
Merged via the queue into linebender:main with commit 638755e Jun 12, 2024
3 checks passed
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

Successfully merging this pull request may close these issues.

3 participants