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

Render filters on display objects #11702

Merged
merged 6 commits into from
Jun 25, 2023
Merged

Conversation

Dinnerbone
Copy link
Contributor

This only hooks up existing filters, does not add new filters implementations.
AVM1 filters remain disconnected from DisplayObject.filters, this does not fix that.

render/wgpu/src/backend.rs Outdated Show resolved Hide resolved
@@ -768,6 +768,7 @@ pub fn render_base<'gc>(this: DisplayObject<'gc>, context: &mut RenderContext<'_
handle: handle.clone(),
commands: offscreen_context.commands,
clear: this.opaque_background().unwrap_or_default(),
filters: this.filters(),
Copy link
Collaborator

Choose a reason for hiding this comment

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

just to be sure: do you think us copying the filters vec (the call does a deep .clone() inside) is okay if dome each time here and in debug code?

Copy link
Contributor Author

@Dinnerbone Dinnerbone Jun 25, 2023

Choose a reason for hiding this comment

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

Debug code: I don't mind perf losses there so much, but I'd like to change filters to return a ref but that felt out of scope (already is vec today) which would be fine for debug

For render: It needs a clone ideally, I don't like the idea of refs leaking out to render, or things that can be interior mutable. I'd like to end up with a state that rendering can be done entirely on another thread on desktop (maybe web in a few years...) and trying to keep that design in mind, it submits work and moves on

Edit: The size of a filter shouldn't be too big, either!

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