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

OPENGL3D: Make use of the active rect provided by WindowedGraphicsManager #3826

Merged
merged 1 commit into from Jun 18, 2022

Conversation

ccawley2011
Copy link
Member

@ccawley2011 ccawley2011 commented Apr 19, 2022

This has two main advantages:

  • The mouse cursor is now restricted to the correct area when grabbing the cursor with SDL 2.0.18 or later. This fixes Trac #13152.
  • 3D games now make use of the selected stretch mode, which replaces the old aspect ratio code. This fixes Trac #12475.

It looks like WindowedGraphicsManager is able to handle games that support arbitrary resolutions without any additional changes. The important thing is to make sure that getWidth() and getHeight() return the correct values when recalculateDisplayAreas() is called.

@ccawley2011 ccawley2011 requested a review from aquadran Apr 19, 2022
@orgads
Copy link
Contributor

@orgads orgads commented Apr 19, 2022

Thanks for the fix.

Looks like the rect calculation is not accurate. With this change, I can move the mouse "below" and "to the right" of the real screen boundaries on my grim-mouse-squashed branch.

@orgads
Copy link
Contributor

@orgads orgads commented Apr 19, 2022

There seems to be a misalignment between the real cursor and the in-game cursor. I found that if I switch out of the full-screen window, then switch back to it, the cursor is synchronized again, and the boundaries are correct.

@orgads
Copy link
Contributor

@orgads orgads commented Apr 19, 2022

I tried commenting a suggestion instead of adding it as a commit, but I couldn't find how, since part of the change is outside the patch context.

If you approve my additional commit, please squash it. Otherwise, please suggest a different fix, apply it and force-push.

@aquadran
Copy link
Member

@aquadran aquadran commented Apr 19, 2022

have you tested with SDL1?

@ccawley2011
Copy link
Member Author

@ccawley2011 ccawley2011 commented Apr 19, 2022

If you approve my additional commit, please squash it. Otherwise, please suggest a different fix, apply it and force-push.

Done, thanks.

have you tested with SDL1?

Good point. I've pushed an additional fix for SDL 1.2 builds.

Copy link
Member

@lephilousophe lephilousophe left a comment

I have two small suggestions. This may simplify compiler work, but except that I think it will produce the same code.

@lephilousophe
Copy link
Member

@lephilousophe lephilousophe commented Apr 25, 2022

Is there any objections against this PR?

@bluegr
Copy link
Member

@bluegr bluegr commented Apr 28, 2022

Is there any objections against this PR?

Looks OK to be merged. However, there were two comments about code readability that may need to be addressed first, so that the code is more readable

@rsn8887
Copy link
Contributor

@rsn8887 rsn8887 commented May 4, 2022

Considering this:
// HACK: SdlGraphicsManager disables the system cursor when the mouse is in the
// active draw rect, however the 3D graphics manager uses it instead of the
// standard mouse graphic.

Maybe related to this, it would be good in the future to not rely on system cursor. There has been a long-standing issue with missing mouse cursor when the F5 menu is brought up in 3d games on non-windowed OS like Switch and Vita that don’t have a system cursor. Even on my RPi style handheld with Batocera Linux the system mouse cursor is wrongly oriented and unusable. I made a bug here https://bugs.scummvm.org/ticket/13464#ticket

This is just a comment to a related issue, please don’t let this slow down any merging of this awesome PR.

@aquadran
Copy link
Member

@aquadran aquadran commented May 7, 2022

@rsn8887 I agree, system cursor support can be dropped in future. 3d engines don't relay on it. only GUI part. I don't recall if we support cursor in 3d backend currently. it was not never implemented in ResidualVM.

@lephilousophe
Copy link
Member

@lephilousophe lephilousophe commented May 7, 2022

It's implemented in Android 3D backend but not in OpenGL 3D SDL.

@orgads
Copy link
Contributor

@orgads orgads commented Jun 7, 2022

Is anything blocking this change from being merged?

@ccawley2011 ccawley2011 merged commit 60c7372 into scummvm:master Jun 18, 2022
8 checks passed
@ccawley2011 ccawley2011 deleted the opengl3d-active-rect branch Jun 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
6 participants