SDL icon indicating copy to clipboard operation
SDL copied to clipboard

Android `setOrientationBis` issues

Open AntTheAlchemist opened this issue 1 year ago • 21 comments

In the case of SDL_SetHint(SDL_HINT_ORIENTATIONS, "LandscapeLeft LandscapeRight Portrait PortraitUpsideDown"), setOrientationBis will decide depending on w > h. This is a bug. It should be using SCREEN_ORIENTATION_FULL_SENSOR instead, to let the sensor decide.

AntTheAlchemist avatar Jul 26 '24 00:07 AntTheAlchemist

Looking at the code, it seems like it will do what you want if the window is resizable, right? Here: https://github.com/libsdl-org/SDL/blob/a880410cb2cc03d9e3b88c358f79f0eb0e7e7c4e/android-project/app/src/main/java/org/libsdl/app/SDLActivity.java#L1142-L1146

slime73 avatar Jul 26 '24 00:07 slime73

Looking at the code, it seems like it will do what you want if the window is resizable, right?

You're right - I didn't notice I was missing that flag in my window creation 🤦🏼‍♂️ Thanks!

AntTheAlchemist avatar Jul 26 '24 00:07 AntTheAlchemist

I did find another bug though. If choosing between a flipped, or not, version of landscape / portrait, the user setting isn't respected, so it will always use the sensor, regardless of user's auto rotate setting. So, if it's a full 360 rotation, that works fine, and respects the user's auto rotate. But landscapeLeft & landscapeRight will always be sensor, ignoring user's auto rotate. Does that make sense?

AntTheAlchemist avatar Jul 26 '24 00:07 AntTheAlchemist

Patches welcome!

slouken avatar Jul 26 '24 01:07 slouken

I'll have a bash at a fix later - it's making me go cross-eyed at the moment.

AntTheAlchemist avatar Jul 26 '24 12:07 AntTheAlchemist

07636ac fixes the user's auto-rotate issue, but I found yet another problem in the logic:

SDL_SetHint(SDL_HINT_ORIENTATIONS, "LandscapeLeft LandscapeRight Portrait"); will allow PortraitUpsideDown, because there's no distinction between one portrait and both landscape (and visa versa... one landscape and both portraits) when deciding to allow all orientations (hope that makes sense). I don't see a way to actually fix this without looking to see if we're attempting to get landscape or portrait. setOrientationBis() is flawed; a different approach is needed.

AntTheAlchemist avatar Jul 27 '24 16:07 AntTheAlchemist

I've noticed issues on Android with this as well. My app basically uses a SDL1/2/3 (using ifdefs) compatible way of creating a SDL surface, and for SDL2/3 the window, texture and attributes. The SDL2 method is based on the 1.2->2.0 migration guide. Then the same is done for the SDL2->SDL3 migration guide. The code (detecting SDL2 and SDL3 defines to differentiate the code) is thus compatible with all 3. There's also a STATICSCREEN setting that changes some specific nonresizing behaviour for unchanging screens (with a fixed horizontal/vertical resolution. Basically all consoles and Android in my app's case). Sony PSP (SDL 1.2) uses a hardcoded resolution, the others are dynamic (Windows/Linux) or static as well as autodetected (SDL2/3, using SDL defaults). See: https://bitbucket.org/superfury/commonemuframework/src/default/emu/gpu/gpu.c function updateWindow (the updateWindowOrientation is also called when the screen orientation changes (SDL orientation event is pretty much given into it's parameters with update set to 1 instead of updateWindow's 0), causing updateWindow to be called at a later point in time to refresh the window size etc., done through forcing the texure to be reloaded (the variable being set to 1)).

Now, I've looked at the results. On Windows, somehow horizontal mouse coordinates go wonky on full screen SDL3 (it almost looks like they're too large movements combined with x/y swapped in the default Landscape mode of most monitors?). Maybe also the coordinates are DPI-affected, making them even smaller incorrectly? My app expects a simple linear float from 0.0-1.0 of the window's relative x/y axis (converting it back to pixels or inches/mm for it's purposes) like in windowed mode. But this doesn't seem to work properly? Maybe it's also affected by the fullscreen letterbox handling using the SDL SDL_SetRenderLogicalPresentation SDL_LOGICAL_PRESENTATION_LETTERBOX to keep aspect ratio proper? Then, on Android, the touch events seem to follow the (hard) configured orientation (top right bottom of current orientation being reported as almost coordinate 1.0,1.0. This works fine on both SDL 3.1.1 and 3.1.3+. However, the screen window (rendered landscape in my app always) is setup as follows: (Works fine on SDL 3.1.1 on Android, faults like below on 3.1.3 and 3.1.6)

  • Before SDL_Init, set the orientation Hint to "LandscapeLeft LandscapeRight" to only support landscape outputs (my app tries to use the full screen, which is better when using the old sensor landscape mode in SDL2 activity, which is removed for SDL3). Disabling this call makes the screen render stretched to edges of the screen in portrait mode only (landscape has no effect). The top-right bottom of the screen always registers as (1,1) with touch events, so that's correct. But since the screen stays stuck on portrait mode, the touch events rotate to 0,90,180,270 degrees even though the screen pixels only rotate 0,180 degrees?
  • I've tried fixing it by making the texture rotate 90 degrees clockwise from within the rendering (https://bitbucket.org/superfury/commonemuframework/src/default/emu/gpu/gpu_sdl.c function safeFlip. It's calling SDL_RenderTextureRotated instead of SDL_RenderTexture in this case (only on Android right now, although commented out using a "#if 0" preprocesor directive). Somehow the screen gets oddly stretched in landscape mode with this?

Edit: Managed to fix the 3.1.3+ (or 3.1.6, don't know when high DPI support was introduced) handling of mouse and touch coordinates. Now, both coordinate x/y locations are first filtered using SDL_ConvertEventToRenderCoordinates. Also when using finger events, it also transforms ms it back to relative coordinates using the know x and y resolution (since the touch subsystem of my app uses that for compatiblity).

Still working on getting the Android version running properly.

superfury avatar Nov 12 '24 21:11 superfury

I've tried to make it run better this time.

I've changed the window to resizable (to properly obtain the four direction events) and observed the height/width gotten from the SDL_GetDisplayUsableBounds to obtain the coordinates for rendering on the window's automatically located display. For some reason, Portrait mode reports a portrait size. And Landscape mode reports a Landscape size (portrait, but x/y swapped). But when rendering the screen, both are rendered in portrait always? And since the window size can't be adjusted using SDL_SetWindowSize (it has no SDL handler when I step into the function (returning SDL_Unmapped()) and doesn't affect the renderer properly). So it's giving a Landscape sized screen which can only be rendered in portrait! Thus messing up all aspect ratios that are used etc. The coordinate system performs fullscreen always in those cases, so at least that's correct for that case. non-Fullscreen (using the same method but for filling the desktop area using a window) desktop windows on Windows also messes up the coordinates using SDL_ConvertEventToRenderCoordinates. It works fine in fullscreen (with letterboxing presentation) and normal non-letterbox presentation windowed mode though?

superfury avatar Nov 15 '24 08:11 superfury

@superfury keep in mind that there are several other bugs at the moment which will confuse things. Vulkan currently renders in portrait only, which might explain what's going on. And changing orientation will leave the rotated renderer in a broken state coordinate-wise until after 1 present. There is no way to reliably determine what's going on until the other bugs are fixed.

AntTheAlchemist avatar Nov 15 '24 15:11 AntTheAlchemist

OK. I found out one weird thing. When on Android, somehow in landscape mode, the width is large while the height is small. So basically, it's providing a viewport of a landscape window, even though the window being rendered is actually portrait (I see it being cut off in this mode, as the horizontal vs vertical sizes are inversed in the window when created without any sizes (for autodetection using the display it's connected to))?

superfury avatar Nov 15 '24 18:11 superfury

OK. I found out one weird thing. When on Android, somehow in landscape mode, the width is large while the height is small. So basically, it's providing a viewport of a landscape window, even though the window being rendered is actually portrait (I see it being cut off in this mode, as the horizontal vs vertical sizes are inversed in the window when created without any sizes (for autodetection using the display it's connected to))?

Ah, can you update to the latest main code and see if that's fixed?

slouken avatar Nov 15 '24 18:11 slouken

Didn't update yet, but quickly looked for related commits since 3.1.6 prerelease.

I did see some function change to a different function call for rendering surfaces, but the main issue is weirder than that.

Basically, from what I can see stepping through the code, the display ID (1) used by the window creation (and all steps including the renderer and texture) without x/y resolution specified reports the horizontal and vertical axes flipped in landscape mode. That basically puts any textures created based on the window out-of-range.

I think it's because for example a 1K x 2K portrait screen reports 1K x 2K in portrait mode (based on orientation events) and 2K x 1K in landscape mode. The issue is that everything is always rendered in portrait mode, even in 'landscape mode'. So it creates a 2K x 1K texture and surface and writes those pixels to a 1K x 2K framebuffer in portrait mode. I see a 4K screen getting (texture rendered at to +90 degrees angle rendering by my app due to landscape mode) rendered with what looks like half the screen vertically centered in landscape mode and the right half of the rendered display being off-screen. So it's like it's mapping the right half out-of-frame (clipping it or corrrupting memory?) and only the top 1/2 of the surface rendered, with roughly 25% black bar above and 25% black bar below (at a ridiculously low resolution)? The renderer is set to use letterboxing and uses SDL_GetDisplayUsableBounds to obtain a resolution to try and set using SDL_setWindowSize (the source of the incorrect axes in landscape mode).

My SDL 1/2/3 compatible code for their compatibility: https://bitbucket.org/superfury/commonemuframework/raw/default/emu/gpu/gpu.c void updateWindow(word xres, word yres, uint_32 flags) ^ Handles screen allocation for rendering

https://bitbucket.org/superfury/commonemuframework/raw/default/emu/gpu/gpu_sdl.c void safeFlip(GPU_SDL_Surface *surface) ^ Handles rendering a surface when it's ready to be displayed.

GPU_SDL_Surface is basically a wrapper for SDL_Surface, with stuff like rendering flags (dirty etc.) and various precalcs to speed up operations (like axis precalcs for resizing a different surface onto it (with or without aspect ratio), based on the SDL_gfx original zoomRGBA functions).

superfury avatar Nov 15 '24 22:11 superfury

The issue is that everything is always rendered in portrait mode, even in 'landscape mode'.

This is what was fixed, can you please try it?

slouken avatar Nov 15 '24 22:11 slouken

I'll check it out when I have time.

Also, is SDL_GetDisplayUsableBounds also fixed to report correctly? I seem to notice stepping through the code on my Android phone that the SDL_setWindowSize function (based on the above function call on 3.1.6) doesn't have a handler installed on _this->SetWindowSize on Android, reaching SDL_Unsupported(), located on line 65 right now ? Is that an issue? I think I remember the same on the SDL_SetWindowPosition call too.

superfury avatar Nov 15 '24 23:11 superfury

Just got the current git commit (3.1.7), adjusted my code to no longer flip or orientate using textures (just back to the old SDL_RenderTexture, using a on-render SDL_CreateTextureFromSurface (and calling SDL_DestroyTexture after rendering, before calling SDL_RenderPresent).

The screen now displays correctly on all orientations (using the current SDL3 commit).

superfury avatar Nov 16 '24 22:11 superfury

The screen now displays correctly on all orientations (using the current SDL3 commit).

Great, thanks for the confirmation!

slouken avatar Nov 16 '24 22:11 slouken

@superfury I'm still seeing issue #11324 on opengles2 with current source. The first present after rotation will draw the frame using distorted coordinates. Landscape to portrait seems to be drawn half-way up the screen. Portrait to landscape is all over the place - cannot tell you what's trying to be drawn. Then everything is drawn okay after the first bad present. You'll miss it visually if doing a present every iteration. I don't know if this will mess up your observations.

AntTheAlchemist avatar Nov 17 '24 03:11 AntTheAlchemist

@AntTheAlchemist I indeed see that happening on current commit with Windows SDL3 builds (both in windowed mode (with letterbox) and fullscreen mode with letterbox). Android otoh looks like it's running without issues, although perhaps with a slowdown (but I'm not sure if that's due to SDL or my own app). It's running as I've programmed the surfaces to do, in both portrait and landscape fullscreen modes.

Edit: Also, yes, the second frame (of anything being drawn) seems to indeed be drawn correctly as it should (matching the mouse coordinates from that poitn onwards. The mouse coordinates ofc working properly since the logical resolution change).

In this case my app's testcase runs either in fullscreen window or normal fullscreen. The mouse coordinates seem fine at all time (matching where the mouse is on the logical screen properly). It's just the rendering on Windows (maybe Linux etc. as well, anything with a windowing system that isn't Android maybe?) that seems to have the weird first frame display issue? And since both SDL methods are unchanged behaviour handling for both creation and first/second frames after creation (creation is creation with logical, renderer and surface, first/second frames are texture updates from pixels, SDL_RenderTexture (SDL_RenderCopy on SDL2) and finished off by SDL_RenderPresent. My app basically uses the method of migration from SDL 1.2 (see it's 1.2 migration guide), then ported forwards toward SDL3. And the source code itself is in that way (using ifdefs) compatible with SDL 1/2/3 (2 and 3 from 2.0.0 or 3.1.1 both upwards compatible using manual defines for missing stuff (in this case it's the SDL_setHint calls that are handled that way, manually defining the hint names if not defined for the supporting SDL versions (either SDL2 or SDL3 or both)). That way you can compile for SDL 2.0 or 3.1.1, then upgrade the SDL2.dll or SDL3.dll to a newer version and get some or all benefits (depending on compatibility with my app).

superfury avatar Nov 17 '24 05:11 superfury

@superfury okay that's strange because I'm only seeing it on Android. Just tried it on a Windows 10 tablet and rotation works okay first frame. I can only say for visual drawing coordinates, not mouse coordinates - not set up a test for that. What's going on here? Android's Vulkan renderer doesn't have this problem, only opengles2.

AntTheAlchemist avatar Nov 17 '24 14:11 AntTheAlchemist

Mouse coordinates are fine from what I can see (using SDL_ConvertEventToRenderCoordinates). The main issue is that the surface when rendered using the SDL1-2-3 conversion process (using below code actually, with SDL3 being defined) is invalidly rendering the first frame after the resolution change.

//SDL2/3!
// Select the color for drawing. It is set to black here.
SDL_SetRenderDrawColor(sdlRenderer, 0x00, 0x00, 0x00, SDL_ALPHA_OPAQUE);
// Clear the entire screen to our selected color.
SDL_RenderClear(sdlRenderer);
// Copy over our display!
if (sdlTexture) //Gotten a valid texture?
{
				SDL_UpdateTexture(sdlTexture, NULL, *getlayerpixels(surface),(get_pixelrow_pitch(surface) << 2)); //Update the texture!

				#ifdef SDL3
				SDL_RenderTexture(sdlRenderer, sdlTexture, NULL, NULL); //Default behaviour!
				#else
				SDL_RenderCopy(sdlRenderer, sdlTexture, NULL, NULL);
				#endif
}

// Up until now everything was drawn behind the scenes.
// This will show the new contents of the window.
SDL_RenderPresent(sdlRenderer);

getlayerpixels(sdllayer) gives a pointer as &sdllayer->pixels, so effectively boils down to sdllayer->pixels passed to SDL_RenderTexture get_pixelrow_pitch(sdllayer) returns sdllayer->pitch divided by 4 if 4 or higher, layer->w otherwise (if it's less than 4, just a safety value, usually doesn't happen as it's using doubleword (32-bit) integer pixels)). In my case sdlsurface is an extension of the sdllater structure, basically a wrapper. so sdllayer->sdllayer is the SDL allocated layer. And there's some precalculated flags and other precalculated stuff used for rendering in my app as well:

typedef struct {
	SDL_Surface *sdllayer; //The surface itself!
	byte flags; //Our flags!
	semaphore_type *lock;

	//Extra stuff for optimizing resizing!
	int *hrowincrements, *vrowincrements;
	uint_32 hrowincrements_precalcs, vrowincrements_precalcs; //Information about source and destination surfaces!
	uint_32 hrowincrements_size, vrowincrements_size; //The size of the horizontal and vertical row increment sizes!
	uint_32 pixelpitch; //Difference of a row of data in the surface, in pixels!
	uint_32 x1; //X1 coordinate to use!
	uint_32 y1; //Y1 coordinate to use!
	uint_32 resizex1; //Resize-based x1!
	uint_32 resizey1; //Resize-based y1!
} GPU_SDL_Surface; //Our userdata!

flags for example contains if the layer is dirty (bit 0) for rendering onto another layer or display, protected against deallocating of sdllayer (bit 1) and the same for sdllayer->pixels (bit 2, used for layers that point to internal buffers that are reused). And h/vrowincrements are precalcs used for the SDL_ZoomSurfaceRGBA call in my framework (a copy adjusted from the original 1.2.x SDL_net, to precalculate the horizontal and vertical pixel positions relative to the destination window, recalculating only when the display aspect relative from the source to destination window changes, used with 1 source and destination surface at most, stored in the destination surface). And x1/y1 is for subwindow positioning inside a larger window (much like what SDL does with it's logical window, but applied to a SDL surface effectively before adding stuff like interfaces (text layers) on top of it).

superfury avatar Nov 17 '24 18:11 superfury

@slouken my android app works in sensorLandscape but somewhere along the way to version 3.2.14 it stopped working. I tracked down the issue to setOrientationBis. The special condition that allowed sensorLandscape to work is that the SDL_HINT_ORIENTATIONS hint is not specified and that width > height. When I checked using a debugger, setOrientationBis is called the first time around using the right screen dimensions and sensorLandscape is set. However, almost immediately, it is called again with width = 1 and height = 1 which sets the orientation to sensorPortrait and screws everything up.

My window creation is as follows:

ctx->window = SDL_CreateWindow(
        "NES Emulator",
        0,
        0,
        SDL_WINDOW_FULLSCREEN
        | SDL_WINDOW_HIGH_PIXEL_DENSITY
    );

My android manifest is set to use sensorLandscape.

I figured this issue is very closely related to my problem.

EDIT:

I was able to force my desired orientation by overriding setOrientationBis but maybe we shouldn't have to do that.

ObaraEmmanuel avatar May 22 '25 12:05 ObaraEmmanuel

I've gone ahead and ignored setOrientationBis() with width/height of 1. I think this takes care of all the issues in this bug. Please open a new bug with details if you're seeing any further issues.

slouken avatar Oct 09 '25 03:10 slouken