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

IOS7: Add new iOSGraphicsManagers for 2D and 3D OpenGL graphics #5065

Merged
merged 17 commits into from Jul 3, 2023

Conversation

larsamannen
Copy link
Contributor

This commit adds the iOSGraphicsManager class which aims to replace the ios7 backend graphic handling.
It's intended to not include the iOSGraphicsManager into the ios7 backend yet. It will be included when changing the ios7 backend to derive from ModularGraphicsBackend.

@larsamannen larsamannen self-assigned this Jun 1, 2023
@larsamannen larsamannen force-pushed the ls_ios_open_gfx_work branch 2 times, most recently from 3bdb7c8 to 50ace51 Compare June 2, 2023 06:41
@criezy
Copy link
Member

criezy commented Jun 3, 2023

At first glance the changes look good. I will try to test it tomorrow.

@larsamannen larsamannen force-pushed the ls_ios_open_gfx_work branch 2 times, most recently from 96afd93 to aaf5e2d Compare June 19, 2023 20:39
@larsamannen larsamannen changed the title IOS: Add iOSGraphicsManager IOS7: Add new iOSGraphicsManagers for 2D and 3D OpenGL graphics Jun 19, 2023
@larsamannen
Copy link
Contributor Author

The PR is updated with commits adding not just a 2D graphics manager but also a 3D graphics manager.
It's quite a massive PR and might be hard to review and should therefore be tested.

There are some known "issues" that will be worked on:

  • In portrait mode the game screen will be rendered in the middle of the screen which is not preferred when keyboard is shown. The game screen should move up to top of screen.
  • The GameList widget in the launcher is too large when running in Debug mode (only affecting devices following Safe Area guidelines)
  • The keyboard is shown in landscape mode if launching ScummVM in landscape mode.
  • Shaders trying to change resolution to 640x480 will fail

@larsamannen
Copy link
Contributor Author

@criezy I've updated the PR with some fixes and now finally resize of the frame depending on orientation and if keyboard is visible or not.
The PR is now mature enough to be merged according to me.
Please review and test when you have the possibility.

@criezy
Copy link
Member

criezy commented Jun 29, 2023

Great job! That's impressive.
I have not looked at the code yet, but I compiled and tested it on my iPad (iOS 12) and it works very nicely.

I have only found one issue: when rotating from landscape to portrait the display is not resized and the keyboard that appears is overlayed on part of the display. Hiding and showing the keyboard again (using the pinch gestures) solves this and the display gets resized.

Just after rotation:
image

After hiding and bringing back the keyboard everything is fine:
image

The same issue also happens in game:
image

image

@criezy
Copy link
Member

criezy commented Jun 29, 2023

Also when in the launcher I think it would be better not to pop up the keyboard when rotating to portrait mode since the GUI can use all the screen. When a game is running however, since it will only use part of the screen I think it still makes sense to automatically pop up the keyboard.

@criezy
Copy link
Member

criezy commented Jun 29, 2023

And I am adding a couple of screenshots for features that work well.

The launcher layout in fullscreen portrait is nice (although I am unsure about having the buttons at the top when using the icons):
image

image

Having access to the shaders is also great:
image

image

@larsamannen
Copy link
Contributor Author

Great job! That's impressive. I have not looked at the code yet, but I compiled and tested it on my iPad (iOS 12) and it works very nicely.

Thanks.

I have only found one issue: when rotating from landscape to portrait the display is not resized and the keyboard that appears is overlayed on part of the display. Hiding and showing the keyboard again (using the pinch gestures) solves this and the display gets resized.

Hmmm, the resizing in the function adjustViewFrameForSafeArea is probably racy with the notifications received for the keyboard. I have 3 devices, one iPad mini ver 1 running iOS 9, one iPhone 12 mini and an iPad 12 Pro 12.9 both running latest version of iOS and I don't get the same problem. But I think I wrote in the commit message that I struggled a bit with the order of events.
What I think happens in that case is when you rotate to portrait mode the keyboard will send the didShow notification before the adjustViewFrameForSafeArea is called by the system. The adjustViewFrameForSafeArea will reset the height to fullscreen if the orientation is changed.
If you could help me debug that it would be fantastic. Perhaps we should treat that as a minor issue which doesn't block this PR?

Also when in the launcher I think it would be better not to pop up the keyboard when rotating to portrait mode since the GUI can use all the screen. When a game is running however, since it will only use part of the screen I think it still makes sense to automatically pop up the keyboard.

I can fix that

The launcher layout in fullscreen portrait is nice (although I am unsure about having the buttons at the top when using the icons)

I agree, I think we should team up with the Android team and create a layout that is better suited for tablets and phones. But that will be another story.

@criezy
Copy link
Member

criezy commented Jun 30, 2023

Perhaps we should treat that as a minor issue which doesn't block this PR?

I agree. For me this is not a blocker.

Copy link
Member

@criezy criezy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall this looks solide. I only made a few comments.

Not automatically showing the keyboard in portrait mode when in the launcher, and fixing the timing issue about the frame resize when rotating can both be done in tree after this is merged. Also as noted in Discord we might want to add an option to force landscape mode for games.

ports.mk Show resolved Hide resolved
backends/platform/ios7/ios7_video.mm Show resolved Hide resolved
backends/graphics/ios/ios-graphics.h Outdated Show resolved Hide resolved
backends/graphics3d/ios/ios-graphics3d.cpp Outdated Show resolved Hide resolved
The ios7 backend implements the graphic handling in the backend code.
iOS supports OpenGL through the OpenGL Framework since iOS 2.0. It's
marked as deprecated but is still shipped with the SDKs for iPhoneOS
and tvOS and will hopefully be so for some time.
The ios7 backend can therefore utilize the OpenGLGraphicsManager to
handle all graphics.

Implement an iOSGraphicsManager class that can be used in the ios7
backend. The iOSGraphicsManager will require some callback functions
in the ios7 backend. createOpenGLContext() will be called to ask the
backend to create an OpenGL context in which the graphic manager can
draw. The function returns the ID of the renderbuffer which shall be
used when creating the framebuffer object this differ iOS from other
platforms). A custom RenderBufferTarget class is added to address
this.

destroyOpenGLContext() will be called to make sure that the old GLES
context is not reused. notifyContextDestroy() does call the function
OpenGLContext.reset() but that will not destroy the context.

refreshScreen() will be called to ask the backend to present the
drawn graphics on the screen. getSystemHiDPIScreenFactor() is called
to get the screen scaling factor. getScreenWidth() and
getScreenHeight() are called to get the width and height of the
surface to draw on.

This commit adds the class but the ios7 backend doesn't make use of
it quite yet. To use it require the ios7 to be a child class of the
ModularGraphicsBackend. That change requires a lot of changes which
will be targeted in separate commits.

Update docportal and github ci worker to only disable the feature
opengl_classic_game since opengl and opengl_shaders are required to
compile the OpenGLGraphicsManager.
Remove all pure virtual functions in OSystem_iOS7 since they are
implemented by ModularGraphicsBackend.

This commit will break the graphics implementation in the ios7
backend and crash due to no OpenGL context created for the
graphicsManager to use.
Implement callbacks to set up OpenGL context, destroy context, get
scale factor and screen sizes. Implement rendering of graphics drawn
by the iOS graphicsManager.

This commit will enable graphics to be shown again. Screen rotation
and mouse movements are still to be adapted.
Previously the mouse position in the view was tracked using the
pointerPosition property. Scaling and relative mosue movements
were calculated in the view using screen properties stored in the
videoContext structure. Now when moving to iOSGraphicsManager all
that handling will be handled by the WindowedGraphicsManager,
which the iOSGraphicsManager inherit.

Rework the input code to send down pure x and y position values,
scaled according to the view content scale factor.

Remove code related to mouse movement that is no longer needed.
@larsamannen
Copy link
Contributor Author

Updated after comments.
I've added a commit dealing with the automatic opening of the keyboard when in game.

When the screen dimension changes, e.g. on rotation of the device,
the graphic manager has to be informed of the new dimension to be
able to resize the surfaces.

To quickly redraw the entire screen, Common::EVENT_SCREEN_CHANGED
event is passed to the event handler.
Delete the old graphic handling in the IOS7 backend which is not
used anymore after implementing iOSGraphicsManager.

The Accelerate framework is not used anymore. The OpenGLGraphics
manager handles the different color formats.
Add a graphic manager rendering 3D graphics for engines supporting
3D games. The manager is implemented using the 3D graphic managers
for Android and SDL as models.

Most probably Android and iOS can share much more of the code, but
that will be a separate work to refactor.
The iOSGraphics3dManager handles resize since the screen dimension
changes on rotation.

Games not supporting arbitary resolutions, e.g. Grim, are rendered
on an intermediate framebuffer with the size requested by the
engine and then rendered to the backbuffer (a framebuffer bound to
the renderbuffer) and stretched to the screen resolution off the
device.

This commit just adds the manager. It will be utilised in next
commit.

Update gitlab ci worker and update documentation.
Implement the same "hacky" way to switch between the 2D and 3D
iOSGraphicsManagers as the Android and SDL backends.

This commit enables 3D capable games to utilise the horse powers
in the GPU to render graohic. Older iPhones and iPads (iPhone 6,
iPad Mini v1) are able to run quite advanced games without any
stuttering if run in Release mode.
If not in "click-and-drag" mode, left mouse button down and up
events are sent on touches ended if the touch lasted less than
250 ms. If the touch lasted longer it was considered as a move
and no button events are sent.

This commit mimic that behaviour in touchpad mode when "click-
and-drag" mode is enabled. The left mouse button down event is
queued for 250 ms. If the touch is dragged within 250 ms it is
considered as a move and the queued mouse button down event is
cacelled. If no movement is made withing 250 ms the queued
mouse button event is processed.
Since iOS 15 or 16 both auto correction and spelling check have to
be disabled to hide the word suggestion list above the keyboard.
It's important that the main frame, displaying the OpenGL rendered
graphics, has the proper dimensions and depending on the device
orientation. It's also important that the frame is not covered by
the iOS keyboard.

This commit calculates the frame size depening on the orientation
and the keyboard status. The keyboard knows its parent view and
can resize it when the keyboard becomes visible or hidden.

There are multiple scenarios where the frame size is changed.
 - When the keyboard is hidden/shown which can be automatically
   change depending on the device orientation
 - If the system demands the keyboard to get visible or hidden
 - When rotating the device
 - When suspending/resuming the application

There can also be combination of the scenarios above, e.g. if
suspending the application in landscape mode and resume it in
portrait mode.

A lot of effort has been put into testing different scenarios to
verify that the screen size becomes correct. However there might
be some scenario which has not been covered.
The Apple TV remote sends touch events of type UITouchTypeIndirect
since it's not touches made on the screen. In iOS touchpads might
send UITouchTypeIndirect touch events as well as mouse move events.

So in iOS we want to block touches of type UITouchTypeIndirect
while in TV OS we want them to be handled.

Touchpad mode for touch events is also required for the Apple TV
remote to work properly. Make sure to disable the possibility to
disable touchpad mode in TV OS.
Open the iOS system keyboard automatically when starting a game and
screen orientation is portrait. If orientation changes to portrait,
open the keyboard automatically only if game is running.
The GRIM define was incorrectly spelled GRIME which cause no shader
files to be included to the project preventing the game to start.

The freescape shader file freescape_triange.vertex also contained
a spelling error preventing the game to start.
@larsamannen
Copy link
Contributor Author

Pushed an update to send kInputScreenChanged on applicationResume to make sure the screen size is updated in the scenario:

  • minimize app in orientation landscape or portrait
  • resume app in a different orientation than above

@larsamannen larsamannen merged commit 6078586 into scummvm:master Jul 3, 2023
8 checks passed
@larsamannen larsamannen deleted the ls_ios_open_gfx_work branch July 3, 2023 19:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants