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

BACKENDS: Handle screen shaking in WindowedGraphicsManager #1778

Merged
merged 1 commit into from Aug 14, 2019

Conversation

@ccawley2011
Copy link
Member

commented Jul 27, 2019

Note that the Vita backend appears to ignore the active DisplayArea, meaning that screen shaking is unlikely to work on the Vita with this PR.

@rsn8887

This comment has been minimized.

Copy link
Contributor

commented Aug 5, 2019

Why break something on Vita that is currently working? I think a workaround for Vita is necessary before breaking game screen shaking on that platform completely.

Is _gameScreenShakeOffset always measured in game pixels, or host pixels? The code seems to jump between the two.

If the desired functionality is just that the surface should be drawn at a vertically displaced position, then that can easily be done on Vita by changing the call vita2d_draw_texture_scale(_vitatex_hwscreen, x, y, sx, sy); to vita2d_draw_texture_scale(_vitatex_hwscreen, x, y+_gameScreenShakeOffset, sx, sy); here:

vita2d_draw_texture_scale(_vitatex_hwscreen, x, y, sx, sy);
maybe with some appropriate scale factor for _gameScreenShakeOffset.

Also, is there an easy way I can test this also on Switch? Is there a game that shakes in the intro, or some demo that shakes, together with a save game I could load?

@bluegr

This comment has been minimized.

Copy link
Member

commented Aug 5, 2019

Also, is there an easy way I can test this also on Switch? Is there a game that shakes in the intro, or some demo that shakes, together with a save game I could load?

QFG2 uses screen shaking at the very first scene of its intro (the genie climbing the mountain, 00:00:19 - 00:00:23):

https://www.youtube.com/watch?v=ZlHJacyqUmo

@criezy

This comment has been minimized.

Copy link
Member

commented Aug 5, 2019

This can also be tested using the testbed engine. Data for this engine is in dists/engine-data/testbed-audiocd-files/, so point ScummVM to this directory to add the "game" to ScummVM. Also the engine is disabled by default, so make sure to enable it (for example with --enable-engine=testbed).

@rsn8887

This comment has been minimized.

Copy link
Contributor

commented Aug 5, 2019

I just tested it. Here's the diff that should be applied to this PR to fix screen shaking on Vita with it:

diff --git a/backends/graphics/psp2sdl/psp2sdl-graphics.cpp b/backends/graphics/psp2sdl/psp2sdl-graphics.cpp
index 5e6afc2a57..aadf01320a 100644
--- a/backends/graphics/psp2sdl/psp2sdl-graphics.cpp
+++ b/backends/graphics/psp2sdl/psp2sdl-graphics.cpp
@@ -297,7 +297,15 @@ void PSP2SdlGraphicsManager::SDL_UpdateRects(SDL_Surface *screen, int numrects,
        sy = (float)h / (float)screenH;
        if (_vitatex_hwscreen) {
                vita2d_start_drawing();
-               vita2d_draw_texture_scale(_vitatex_hwscreen, x, y, sx, sy);
+               int shakeOffset = _gameScreenShakeOffset * sy * _videoMode.scaleFactor;
+               int bottomBlackBarHeight = dispH - y - h - shakeOffset;
+               if (shakeOffset > 0) {
+                       vita2d_draw_rectangle(0, 0, dispW, shakeOffset, RGBA8(0, 0, 0, 255));
+               }
+               if (bottomBlackBarHeight > 0) {
+                       vita2d_draw_rectangle(0, y + h + shakeOffset, dispW, bottomBlackBarHeight, RGBA8(0, 0, 0, 255));
+               }
+               vita2d_draw_texture_scale(_vitatex_hwscreen, x, y + shakeOffset, sx, sy);
                vita2d_end_drawing();
                vita2d_swap_buffers();
        }
@rsn8887

This comment has been minimized.

Copy link
Contributor

commented Aug 5, 2019

I also tested screen shaking on Switch with this PR, and it works there.

@sluicebox

This comment has been minimized.

Copy link
Member

commented Aug 6, 2019

I've been slowly working on implementing horizontal shaking in addition to vertical. This has been an SCI TODO for a while. Without it, some scenes don't shake. Applied at the same time you get diagonal shake, which SCI also uses. I'd be surprised if no other engines had horizontal shaking.

Conceptually it's simple: change the shake interface from the one y offset to an x and a y, but since that goes through a lot of layers and is in each backend it touches a lot of files, including the core SDL drawing code, and so it's slow going.

I bring that up to ask @ccawley2011, could you explain the benefit of this change? Do you think it would make my thing easier, harder, etc? I'm not challenging it, I'm just unfamiliar with this area and some context would help me.

Alternatively, if anyone faster than me wants to take on implement horizontal shaking, please do! =)

@ccawley2011 ccawley2011 force-pushed the ccawley2011:shake-pos branch from d890634 to ff57d27 Aug 11, 2019

@bluegr

This comment has been minimized.

Copy link
Member

commented Aug 12, 2019

@ccawley2011: Thanks for addressing the comments in this PR, however you also need to apply the PS Vita patch (posted above) before this PR can be merged

@m-kiewitz

This comment has been minimized.

Copy link
Contributor

commented Aug 12, 2019

I've been slowly working on implementing horizontal shaking in addition to vertical. This has been an SCI TODO for a while. Without it, some scenes don't shake. Applied at the same time you get diagonal shake, which SCI also uses. I'd be surprised if no other engines had horizontal shaking.

Yes, I never implemented it in SCI, because I wanted a proper system implementation like this here.
Original SCI changed a few VGA registers if I remember correctly (I could be wrong though). At least I know that such shakes were possible by changing a few VGA ports, no pixels had to be copied around. Of course that's not possible nowadays anymore.

@Kawa-oneechan

This comment has been minimized.

Copy link
Contributor

commented Aug 12, 2019

Original SCI changed a few VGA registers if I remember correctly (I could be wrong though). At least I know that such shakes were possible by changing a few VGA ports, no pixels had to be copied around. Of course that's not possible nowadays anymore.

You're quite right. I'm looking at my disassembly of VGA320.DRV right now and the Shake routine does mess with the CRTC registers.

@rsn8887

This comment has been minimized.

Copy link
Contributor

commented Aug 14, 2019

@bluegr The Vita patch is not needed anymore. As of #1803 , the Vita now uses _activeArea rect, just like the other platforms. Thanks @ccawley2011 for pointing this out to me.

@bluegr

This comment has been minimized.

Copy link
Member

commented Aug 14, 2019

Right, since the Vita patch is no longer needed, this PR can be merged. Thanks for your work!

@bluegr

bluegr approved these changes Aug 14, 2019

@bluegr bluegr merged commit 1feb86e into scummvm:master Aug 14, 2019

2 checks passed

Codacy/PR Quality Review Up to standards. A positive pull request.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@ccawley2011 ccawley2011 deleted the ccawley2011:shake-pos branch Aug 14, 2019

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