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

Overhaul cursor loading #17517

Merged
merged 1 commit into from Jan 4, 2020
Merged

Overhaul cursor loading #17517

merged 1 commit into from Jan 4, 2020

Conversation

@pchote
Copy link
Member

pchote commented Dec 29, 2019

This PR reworks the way we load and render cursors, adding support for 24/32bit artwork and improving cross-platform compatibility for hardware cursors.

In particular, this fixes a relatively recent macOS issue (possibly introduced with 10.15) where relative offsets between frames in an animation seem to be ignored, making them jump around incorrectly. If we are lucky, this may also finally solve #10172. See commit messages and code comments for more details.

Depends on #16885.

@dragunoff

This comment has been minimized.

Copy link
Contributor

dragunoff commented Dec 29, 2019

Just tested this. The cursors render but I am seeing some RGBA/BRGA weirdness with the select cursor - it is using the same sequence as the default cursor but it seems to be swapping the blue/red channels when nothing is selected:
rgba-cursor-weirdness

The same happens with attack ground and a-move:
attack-cursor

EDIT: Only happens with hardware cursors.

@dragunoff

This comment has been minimized.

Copy link
Contributor

dragunoff commented Dec 29, 2019

Also notice how the cursor is skewed and distorted in the screen capture above. This only affects hardware cursors and is only visible in the screen capture. The correct cursor size can be seen in the gif below (captured with software cursors on). This could be due to some weird interaction with the screen capturing software.

The gif below also illustrates that the setting to increase the command cursor size doesn't work well when the default cursor is the same as the select cursor (but that seems like a separate issue):
17i6xa3LC9

@pchote

This comment has been minimized.

Copy link
Member Author

pchote commented Jan 3, 2020

The mouse cursor issues that this fixes on macOS are rather annoying, so I will separate the platform workarounds from the RGBA support (which can go back into #16885, inverting the PR dependency) so we can take this for the hotfix.

We should pick 010fafc to prep too.

@pchote pchote force-pushed the pchote:cursor-overhaul branch 2 times, most recently from 49aa32b to 6b7824f Jan 3, 2020
@pchote pchote changed the title Overhaul cursor loading and rendering Overhaul cursor loading Jan 3, 2020
@pchote

This comment has been minimized.

Copy link
Member Author

pchote commented Jan 3, 2020

Updated. This now includes only the HardwareCursor fixes, and picks cleanly to prep.

Copy link
Member

Mailaender left a comment

I spot no regressions on Linux.

All frames in a sequence now use the same bounds
and hotspot, and have a size that is a multiple of 8.
@pchote pchote force-pushed the pchote:cursor-overhaul branch from 6b7824f to 0aa0a15 Jan 3, 2020
@pchote

This comment has been minimized.

Copy link
Member Author

pchote commented Jan 3, 2020

Updated.

Copy link
Member

abcdefg30 left a comment

#10172 seems to no longer happen when starting D2k, but still happens around every second time starting TS.

@abcdefg30

This comment has been minimized.

Copy link
Member

abcdefg30 commented Jan 4, 2020

Still merging since fixing #10172 was not the primary goal here.

@abcdefg30 abcdefg30 merged commit 439cd4a into OpenRA:bleed Jan 4, 2020
2 checks passed
2 checks passed
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@abcdefg30

This comment has been minimized.

Copy link
Member

abcdefg30 commented Jan 4, 2020

@pchote pchote deleted the pchote:cursor-overhaul branch Jan 12, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.