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

core: Make applyFilter with zero-sized rects noops #17965

Merged
merged 1 commit into from
Sep 25, 2024

Conversation

adrian17
Copy link
Collaborator

@adrian17 adrian17 commented Sep 17, 2024

Verified that it fixes #17964 . Hopefully should fix most/all of "dimension X is zero" in applyFilter.

I don't know whether this is the best place (it can be done a layer above and below, and after more clamping). @Dinnerbone can you please check it out?

@adrian17 adrian17 changed the title render: Make applyFilter with zero-sized rects noops core: Make applyFilter with zero-sized rects noops Sep 17, 2024
@adrian17 adrian17 added A-rendering Area: Rendering & Graphics T-fix Type: Bug fix (in something that's supposed to work already) labels Sep 17, 2024
Copy link
Contributor

@Dinnerbone Dinnerbone left a comment

Choose a reason for hiding this comment

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

Looks good to me, thank you!

I was initially concerned that we might not catch things that are effectively zero, but I think in practice filters grow the size so an effective zero might end up being a positive value, and I think our rendering should handle that fine as long as the source size isn't zero; so this should be good, I think!

@adrian17
Copy link
Collaborator Author

On hold until the Discord questions are resolved.

@torokati44 torokati44 marked this pull request as draft September 18, 2024 08:27
@torokati44
Copy link
Member

(converted to draft accordingly)

@adrian17 adrian17 marked this pull request as ready for review September 18, 2024 18:57
@adrian17 adrian17 force-pushed the noop-zero-applyfilter branch 3 times, most recently from de83609 to 472685c Compare September 18, 2024 18:59
@n0samu
Copy link
Member

n0samu commented Sep 22, 2024

Strangely, #17964 seems to have been deleted. I've archived it from Google's cache for future reference: https://archive.is/szqJP

@danielhjacobs danielhjacobs enabled auto-merge (rebase) September 25, 2024 16:19
@danielhjacobs danielhjacobs merged commit 53b6cda into ruffle-rs:master Sep 25, 2024
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-rendering Area: Rendering & Graphics T-fix Type: Bug fix (in something that's supposed to work already)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants