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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

[d3d9] Add a software cursor emulation config option #3841

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

WinterSnowfall
Copy link
Contributor

@WinterSnowfall WinterSnowfall commented Feb 5, 2024

In relation to #3020. The game tries to create a 64 x 64 cursor, which wasn't supported prior to this PR, but is otherwise happy with our existing code path catering for hardware cursors. It even happily switches cursor icons in game on hover over certain objects.

Since this isn't exactly a software cursor implementation, rather a workaround, I've created a config option for it.

As far as I know this is the only d3d9 game that actually depended on software cursor support and this PR is enough to get it working properly. I can't guarantee other games will work just as well, since I haven't done a lot of validations and exotic bitmap sizes may cause issues. But we can improve the logic a bit if that ever happens to be a problem.

I welcome any testing, but this shouldn't affect the previous hardware cursor logic (unless I messed up badly somewhere, which doesn't seem to be the case based on my tests at least 馃槄).

Would also appreciate very critical pairs of eyes on my memory allocations and use of pointers, since it's not something I get to do every day (and frankly I'm getting too old for it). Or I'm open to better alternatives, if any exist.

@WinterSnowfall WinterSnowfall force-pushed the swcursoremul branch 2 times, most recently from b6724b7 to 4ad9634 Compare February 6, 2024 00:44
@allfoxwy
Copy link

allfoxwy commented Feb 6, 2024

I happened to make a very close pull to yours recently: #3828 #3827

I vote for this pull that it would be better to support variable cursor size rather than hard-code as 32x32. I did some test on Windows 11 and Xfce + X11 both of them being able to display a 64x64 cursor.

And FYI, from what I read from MS document, cursor size API should have a fallback capability scaling the size back to 32x32 when the underlying platform can not support different size. I didn't write such code.

@WinterSnowfall
Copy link
Contributor Author

I happened to make a very close pull to yours recently: #3828 #3827

Indeed, saw those. Together with @Blisto91's regular mentions about implementing a software cursor (which, ironically, I still haven't), they reminded me there's a game that actually cares about these things. To be fair, my main objective here is to fix Dungeon Siege 2, so it's far less broad in scope.

I vote for this pull that it would be better to support variable cursor size rather than hard-code as 32x32. I did some test on Windows 11 and Xfce + X11 both of them being able to display a 64x64 cursor.

I'm on MATE + X11 and it also works fine.

And FYI, from what I read from MS document, cursor size API should have a fallback capability scaling the size back to 32x32 when the underlying platform can not support different size. I didn't write such code.

A good point, but that's still not being done. The hardware cursor path always assumes a 32 x 32 cursor regardless of bitmap size and does no scaling, which has never been a problem really, so it's probably good enough. Getting Dungeon Siege 2 to display a cursor was actually a lot less work initially, however the cursor was getting clipped to the top left 32 x 32, which I also had to address.

@Blisto91
Copy link
Contributor

Blisto91 commented Feb 7, 2024

Confirmed on Linux and Windows in Dungeon Siege II from GoG

@WinterSnowfall
Copy link
Contributor Author

Confirmed on Linux and Windows in Dungeon Siege II from GoG

Thanks for testing and confirming, especially on Windows!

@WinterSnowfall
Copy link
Contributor Author

WinterSnowfall commented Feb 9, 2024

I've added one more... well, I'd like to say "elegant fix", but we only have "cursed hack" on the menu today... in order to make the cursor emulation work properly with Castle Strike, a d3d8 game that also tries to create a 64 x 64 hardware cursor, but then also tries to control it with win32 calls (don't ask). Retested everything with Dungeon Siege II, which doesn't seem to mind any of the changes.

I'll PR the config option for Castle Strike against d8vk, since it's a d3d8 game.

P.S.: I'll wait for some Windows testing by the grace of @Blisto91, after which I'll un-draft it again.

@WinterSnowfall
Copy link
Contributor Author

WinterSnowfall commented Feb 9, 2024

There's a bit of a hitch on Windows with Castle Strike, since the cursor won't show initially, rather only after alt+tabbing out and back in, but that bit is probably due to some events we're not handling. It works fine in Wine/Proton at least, and Dungeon Siege 2 is good on both Wine/Proton and Windows.

There's some minor differences when compared to WineD3D's software cursors (namely the cursor stays on screen during loading screens for Castle Strike), but a somewhat janky cursor is better than no cursor. It also seems to hide/show just fine in-game, so that's good.

I've run out of ideas on how to make this any better without actually implementing a software cursor, which would be a major pain I'm not sure anyone is willing to pursue, so I'll leave it as is.

@WinterSnowfall WinterSnowfall force-pushed the swcursoremul branch 4 times, most recently from f4b239d to 7117ec6 Compare March 8, 2024 22:18
@WinterSnowfall
Copy link
Contributor Author

I know this PR isn't anything groundbreaking, but it would be nice to have it merged at some point, pretty please 馃檹 . Unless there's anything left to address or you have any objections to it.

@K0bin
Copy link
Collaborator

K0bin commented May 6, 2024

I don't quite get why this is a config option. Is there any downside to always enabling/exposing this?

@WinterSnowfall
Copy link
Contributor Author

WinterSnowfall commented May 6, 2024

I don't quite get why this is a config option. Is there any downside to always enabling/exposing this?

There's two reasons for that mostly:

  • Hardware cursors are only ever supposed to be 32 x 32 pixels, especially in windowed mode. I've done some tests out of curiosity and it looks like WineD3D validates this, so we're going out of spec to make some games happy without actually implementing software cursors. This path is essentially a workaround for the absence of 64 x 64 pixels icon support and mouse input message handling in the case of hardware cursors. Only 2 games out there that we know of would need it to work properly.
  • This bit is a rather cursed hack done exclusively for Castle Strike, which relies on some cursed form of win32 message handling to show the cursor on its main menu (then it show/hides it properly with d3d8 calls during gameplay). I would not hijack the window cursor pointer for every well behaving game out there.

@K0bin
Copy link
Collaborator

K0bin commented May 6, 2024

Honestly, why don't we just implement software cursor?

@WinterSnowfall
Copy link
Contributor Author

Honestly, why don't we just implement software cursor?

... go ahead :). This PR is mostly a stopgap until we get there and this path should be removed once we do. That being said, the window message handling bit sounds like a potential major pain to get right (and a bit beyond what I'd venture to implement).

@WinterSnowfall
Copy link
Contributor Author

I'm no longer convinced it's worth resorting to such hackery, rather we should aim to properly implement software cursors, as @K0bin mentioned. I'll keep this PR around until that's done to potentially cherry pick (for a new PR) some minor fixes I did on the hardware cursor path (in case they're not addressed at that point).

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.

None yet

5 participants