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

Pass RenderLayers to VelloCanvas #71

Merged

Conversation

ChristopherBiscardi
Copy link
Contributor

This is the fix I talked about in #70 and the PR that would be merged (the other should be closed)

This PR adds a config option to VelloPlugin and VelloRenderPlugin that can control the RenderLayers that the VelloCanvas is on. This allows fully taking advantage of RenderLayers as before the canvas was rendering on the default layer, layer 0, which meant any camera that wanted to render it also had to render everything else on the default layer, layer 0.

This PR adds a config option to `VelloPlugin` and `VelloRenderPlugin` that can control the `RenderLayers` that the `VelloCanvas` is on. This allows fully taking advantage of `RenderLayers` as before the canvas was rendering on the default layer, layer 0, which meant any camera that wanted to render it also had to render everything else on the default layer, layer 0.
Copy link
Member

@simbleau simbleau left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks mostly good!

src/render/plugin.rs Outdated Show resolved Hide resolved
@ChristopherBiscardi
Copy link
Contributor Author

swapped to VelloRenderSettings Resource for configuration 👍🏼

@simbleau
Copy link
Member

simbleau commented Aug 3, 2024

Thank you for that! I have a slightly different vision where the settings are not an optional resource. I'll apply that directly to your branch. :)

@ChristopherBiscardi
Copy link
Contributor Author

sounds good, feel free to take it away.

@simbleau simbleau enabled auto-merge August 4, 2024 00:31
@simbleau simbleau added this pull request to the merge queue Aug 4, 2024
Merged via the queue into linebender:main with commit f21cdc2 Aug 4, 2024
3 checks passed
@ChristopherBiscardi
Copy link
Contributor Author

@simbleau heads up that the way the Resource is inserted in the new changes here forces consumers to insert the resource after the plugin, or it will silently throw the user's Resource away and use the defaults instead.

github-merge-queue bot pushed a commit that referenced this pull request Aug 4, 2024
Addresses some friction where the `VelloRenderer` was not a resource and
required Locals temporarily. This was mainly observed in the `cube3d`
example. Likewise, the rendering params such as `use_cpu` and
`antialiasing` were not configurable and the use of `RenderLayers` were
cleaned up after @ChristopherBiscardi brought up some comments after
#71.

@ChristopherBiscardi Please let me know if you still have concerns with
RenderLayers.
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.

2 participants