Project

General

Profile

Actions

Emulator Issues #10770

closed

Android: Use DialogFragment to handle screen rotation in NativeLibrary.displayAlertMsg

Added by ryanebola16 over 6 years ago. Updated almost 4 years ago.

Status:
Fixed
Priority:
Normal
Assignee:
% Done:

0%

Operating system:
Android
Issue type:
Bug
Milestone:
Regression:
No
Relates to usability:
No
Relates to performance:
No
Easy:
No
Relates to maintainability:
No
Regression start:
Fixed in:
5.0-12734

Description

Probably related to #10725

What steps will reproduce the problem?

  1. Rotate the screen when the attached prompt appears for importing WADs. Tested with multiple games.

Not reproducible before 5.0-6164 since prompts could not be answered.

Is the issue present in the latest development version? For future reference, please also write down the version number of the latest development version.

5.0-6164

What are your PC specifications? (CPU, GPU, Operating System, more)

Samsung Galaxy Note8 (SM-N950U)
Qualcomm Snapdragon 835
Octa-core (4x2.35 GHz Kryo & 4x1.9 GHz Kryo)
Adreno 540
6GB RAM (LPDDR4)
Baseband version N950USQS2BQK2
Kernel version 4.4.21
Android 7.1.1


Files

Screenshot_20180110-152853.jpg (107 KB) Screenshot_20180110-152853.jpg ryanebola16, 01/10/2018 09:14 PM
logcat506392.txt (22 KB) logcat506392.txt ryanebola16, 03/08/2018 12:22 PM

Related issues 1 (0 open1 closed)

Has duplicate Emulator - Emulator Issues #11893: Android: Panic alerts don't properly fit screen after rotatingDuplicate

Actions
Actions #1

Updated by leoetlino over 6 years ago

  • Subject changed from Android: Rotating screen while prompted to import WAD causes freeze to Android: Rotating screen when panic alert is shown causes freeze
  • Operating system Android added
  • Operating system deleted (N/A)
Actions #2

Updated by ryanebola16 over 6 years ago

Possibly related to #10815

Actions #3

Updated by ryanebola16 over 6 years ago

This information from Logcat may help solve the problem. Log recorded in Dolphin 5.0-6392.

Actions #4

Updated by ryanebola16 almost 6 years ago

For future reference, #11424 generates a panic alert to test this issue.

Actions #5

Updated by JMC4789 almost 6 years ago

  • Status changed from New to Fixed

We've fixed some auto-rotate issues, but, more importantly, disabled auto-rotation by default because drivers don't like it with our workload. While there are probably still some rotate bugs, users won't run into them unless they change the default settings.

Actions #6

Updated by JosJuice almost 6 years ago

  • Status changed from Fixed to New

We've only disabled portrait mode by default. You can still rotate between the two landscape modes.

Setting to New until this actually is tested.

Actions #7

Updated by JMC4789 almost 6 years ago

I've been testing this pretty much all day, and I've crashed Dolphin a few times with GPU desyncs. Since it doesn't rotate, I haven't had it just instantly quit on me.

Actions #8

Updated by ryanebola16 almost 6 years ago

I just has this issue occur in 5.0-9224 so it should remain open.

Actions #9

Updated by ryanebola16 over 4 years ago

  • Subject changed from Android: Rotating screen when panic alert is shown causes freeze to Android: Use DialogFragment to handle screen rotation in NativeLibrary.displayAlertMsg
  • Status changed from New to Accepted

In NativeLibrary.displayAlertMsg, Android doesn't like locks being used during screen rotation with AlertDialog. Use DialogFragment to handle screen rotation.

See:
https://stackoverflow.com/questions/7557265/prevent-dialog-dismissal-on-screen-rotation-in-android
https://stackoverflow.com/questions/7977392/android-dialogfragment-vs-dialog/21032871#21032871

Actions #10

Updated by ryanebola16 over 4 years ago

Actions #11

Updated by ryanebola16 over 4 years ago

  • Status changed from Accepted to Work started
  • Assignee set to ryanebola16

Two questions:

  1. Instantiation was originally disallowed in NativeLibrary. I assume this is because it wasn't necessary rather than doing so causes problems. Is it alright if I allow instantiation to let DialogFragment handle AlertDialogs during rotation?

  2. A question I made regarding the lock object: https://stackoverflow.com/questions/61963048/android-lock-object-causes-deadlock-when-changing-orientation

Actions #12

Updated by JosJuice over 4 years ago

Instantiation was originally disallowed in NativeLibrary. I assume this is because it wasn't necessary rather than doing so causes problems. Is it alright if I allow instantiation to let DialogFragment handle AlertDialogs during rotation?

Assuming you mean that instantiation of NativeLibrary is disallowed (I wasn't sure what you meant at first), then... I don't really see how allowing that would make sense. Could you explain further about in what way being able to create instances of NativeLibrary would help?

Actions #13

Updated by ryanebola16 over 4 years ago

JosJuice wrote:

Instantiation was originally disallowed in NativeLibrary. I assume this is because it wasn't necessary rather than doing so causes problems. Is it alright if I allow instantiation to let DialogFragment handle AlertDialogs during rotation?

Assuming you mean that instantiation of NativeLibrary is disallowed (I wasn't sure what you meant at first), then... I don't really see how allowing that would make sense. Could you explain further about in what way being able to create instances of NativeLibrary would help?

Sorry, still learning a lot of terminology here. I've made NativeLibrary extend DialogFragment. From what I've read, DialogFragment's dialog can be retained during orientation change by including a newInstance of the class.

In this case, I would use

  private static NativeLibrary newInstance(String title, String message, boolean yesNo)
  {
    NativeLibrary nativeLibrary = new NativeLibrary();

    Bundle args = new Bundle();
    args.putString(ARG_TITLE, title);
    args.putString(ARG_MESSAGE, message);
    args.putBoolean(ARG_YES_NO, yesNo);
    nativeLibrary.setArguments(args);

    return nativeLibrary;
  }

...in NativeLibrary which is a problem if instantiation is disallowed. Allowing instantiation fixes this problem and (as far as I can tell) causes no additional problems. I just wanted to confirm that allowing instantiation would be fine.

If my terminology is still confusing, I can post a draft PR. I was hoping to fix my second question before doing so.

Actions #14

Updated by JosJuice over 4 years ago

But why do you want to instantiate NativeLibrary instead of creating a new class that you can instantiate? My main problem with this is that making NativeLibrary inherit from DialogFragment doesn't make logical sense – the native library as a whole is not a dialog.

Actions #15

Updated by ryanebola16 over 4 years ago

Creating a new class works, thanks! Now I just need to figure out how to get the lock object working with orientation changes.

Actions #16

Updated by JosJuice over 4 years ago

Could you post a stack trace of what the UI thread is doing when you get the deadlock? Perhaps the problem is not with the msgAlert code itself, but with the emulation activity trying to do something like notifying the native library of a new surface.

Actions #17

Updated by ryanebola16 over 4 years ago

No exception is thrown so I manually paused emulation. I'm not sure if this is relevant but after orientation change for the new class that extends DialogFragment, I can hit a breakpoint at onSaveInstanceState but the app is frozen before I can hit a breakpoint at onStop. setRetainInstance didn't seem to matter. I've yet to look into surfaces but let me know if that's what this stack trace indicates.

WaitUntilDoneBooting:-1, NativeLibrary (org.dolphinemu.dolphinemu)
clearSurface:389, EmulationFragment$EmulationState (org.dolphinemu.dolphinemu.fragments)
surfaceDestroyed:232, EmulationFragment (org.dolphinemu.dolphinemu.fragments)
updateSurface:621, SurfaceView (android.view)
windowStopped:220, SurfaceView (android.view)
setWindowStopped:1283, ViewRootImpl (android.view)
setStoppedState:611, WindowManagerGlobal (android.view)
performStop:7144, Activity (android.app)
performDestroyActivity:4395, ActivityThread (android.app)
handleDestroyActivity:4456, ActivityThread (android.app)
handleRelaunchActivity:4730, ActivityThread (android.app)
-wrap18:-1, ActivityThread (android.app)
handleMessage:1599, ActivityThread$H (android.app)
dispatchMessage:105, Handler (android.os)
loop:164, Looper (android.os)
main:6541, ActivityThread (android.app)
invoke:-1, Method (java.lang.reflect)
run:240, Zygote$MethodAndArgsCaller (com.android.internal.os)
main:767, ZygoteInit (com.android.internal.os)
Actions #18

Updated by JosJuice over 4 years ago

Yep, that's the emulation activity getting a deadlock when trying to notify the native library of a surface change. That's annoying... Though on the other hand, if I'm understanding this deadlock correctly, it only happens if we get a msgAlert while the core is in the process of booting.

Actions #19

Updated by ryanebola16 over 4 years ago

...And I thought this was going to be a simple fix. I'll read up on surfaces and return to this at some point in the future. Thanks again!

Actions #20

Updated by ryanebola16 over 4 years ago

I believe if a msgAlert happened when the core was in the process of booting, we'd dereference a nullptr when the surface is destroyed due to EmulationFragment.clearSurface. Instead we get a deadlock so I don't think this only happens if we get a msgAlert while the core is in the process of booting.

Actions #21

Updated by ryanebola16 over 4 years ago

  • Status changed from Work started to Fix pending
Actions #22

Updated by JosJuice almost 4 years ago

  • Status changed from Fix pending to Fixed
  • Fixed in set to 5.0-12734
Actions

Also available in: Atom PDF