Skip to content

Implement BitmapData.scroll() #2190

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

Merged
merged 1 commit into from
Jan 2, 2021

Conversation

torokati44
Copy link
Member

@torokati44 torokati44 commented Dec 31, 2020

This makes the right-scrolling background particles of z0r.de/7463 work, fixing 1/3 of #2073.

I've tested this code manually with a couple different parametrizations, the visual results always exactly matched that of Adobe Flash Player.
I'm not sure if any of this is supposed to be added as some actual test cases, and if so, exactly how - I personally used mtasc to build the .swfs.

Performance-wise I tried to avoid any redundant range checks (hence the unwrap), and conversions to/from premultiplied alpha format.
(There might be faster ways to do this, especially if there is something of a memcpy-like intrinsic on Vecs of simple types, with selectable operation order, but I'm not aware of anything like that (yet), nor do I think it is that critical. [Perhaps the compiler recognizes some data access patterns even, and optimizes the pixel-wise iteration into something more batched, but I haven't checked the generated code.])

Since I'm not that experienced with Rust, I struggled a bit with conditionally reversing the coordinate ranges (and reusing the one for the x dimension, within the y loop), that's why there is some clumsy setup with the Iterator trait object bindings in there. If there is a simpler way to do this, I'd be happy to hear about it.

BTW, @CUB3D, may I ask, what was the reason for putting 32 in the name of some of the pixel-manipulating functions on BitmapData? To me, it seems that the 'exact pair' (with the most direct access to the pixels field) of get_pixel_raw is set_pixel32_raw, and I found this inconsistency slightly strange.

@Herschel
Copy link
Member

Herschel commented Dec 31, 2020

Thank you and welcome!

I think the easiest way to do the reversing coordinate range is to use a while loop: while y != y_max { ... y += dy; } or similar. Feels a little oldschool, but should be the most straightforward way. This should also be more performant as the trait objects may involve an indirect function call.

unwrap doesn't really avoid the bounds check, it moves the bounds check inside the unwrap. Eventually if we want to maximize performance, BitmapData could have unsafe get_pixel_unchecked methods that elide the bounds checks. But I think unwrap or if let Some is fine for now (I prefer to get things working first, and then we can benchmark and see what the actual issues are).

I agree about the pixel32 method naming being a little inconsistent.

@torokati44
Copy link
Member Author

Thanks for the feedback! I'll make a revision ... soon.

@torokati44
Copy link
Member Author

How about now?

@Herschel
Copy link
Member

Herschel commented Jan 2, 2021

Thank you!

@Herschel Herschel merged commit 3b14437 into ruffle-rs:master Jan 2, 2021
@torokati44 torokati44 deleted the bitmapdata-scroll branch January 2, 2021 13:20
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