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

BLADERUNNER: Remove use of unaligned memory access #1839

Merged
merged 1 commit into from Sep 12, 2019

Conversation

@ccawley2011
Copy link
Member

commented Sep 11, 2019

This fixes a crash on RISC OS, where unaligned memory access is not available.

@peterkohaut
Copy link
Contributor

left a comment

why is this needed?
surfaces should be in platform specific format already

i have to check first if this has any impact on performance in nightly builds as we got few reports about them being slow already

@ccawley2011

This comment has been minimized.

Copy link
Member Author

commented Sep 11, 2019

surfaces should be in platform specific format already

That's not the issue. The problem is that commit a7399c5 introduces unaligned memory access, which is unavailable on classic ARM-based systems (and is unavailable or slow on a number of other architectures), which will result in a crash if a 16-bit pixel format is selected (on RISC OS, BGR565 is used as the default pixel format).

@ccawley2011 ccawley2011 force-pushed the ccawley2011:bladerunner-unaligned-mem branch from 80b18af to 82540c7 Sep 11, 2019

@bluegr

This comment has been minimized.

Copy link
Member

commented Sep 12, 2019

The changes look good, and using the macros yields a safer (regarding alignment) and cleaner code. +1 from me

@bluegr
bluegr approved these changes Sep 12, 2019

@peterkohaut peterkohaut merged commit c786b13 into scummvm:master Sep 12, 2019

1 of 2 checks passed

Codacy/PR Quality Review Codacy was unable to analyse your pull request.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@ccawley2011 ccawley2011 deleted the ccawley2011:bladerunner-unaligned-mem branch Sep 12, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.