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

RenderTexture recovery from volatile texture cache results in vertically flipped image and other issues #2163

Closed
rh101 opened this issue Sep 18, 2024 · 5 comments · Fixed by #2166
Labels
bug Something isn't working
Milestone

Comments

@rh101
Copy link
Contributor

rh101 commented Sep 18, 2024

  • axmol version:
  • devices test on: Any Android device
  • developing environments
    • NDK version: r23c
    • Xcode version: 14.2+
    • Visual Studio:
      • VS version: 2022 (17.9+)
      • MSVC version: 19.39+
      • Windows SDK version: 10.0.22621.0+
    • cmake version:
      Steps to Reproduce:
  1. Set preserve EGL context to false via this.mGLSurfaceView.setPreserveEGLContextOnPause(false); in AxmolActivity.java
  2. Add a RenderTexture as a child to the scene, render something to it and display it.
  3. Put the app in the background by switching to another application on the device
  4. Switch back to the test app with the render texture
  5. Notice that the render texture is now flipped
  6. Put the test app into the background again then switch back to it
  7. Notice that now the content of the render texture disappears complete

Two separate issues.

The first issue is that an image is created from the render texture in order for it to be saved in the cache, but the texture data read via glReadPixels results in the flipped image.

The second issue is that that if the texture disappears if the application is put into the background more than once, meaning the EGL context loss event happens more than once.

Example of original RenderTexture output:
image

After first recovery from context loss:
image

After another recovery, it's complete blank.

@rh101
Copy link
Contributor Author

rh101 commented Sep 18, 2024

In this code:

void RenderTexture::listenToBackground(EventCustom* /*event*/)
{
#if AX_ENABLE_CACHE_TEXTURE_DATA
    // to get the rendered texture data
    auto func = [&](RefPtr<Image> uiTextureImage) {
        if (uiTextureImage)
        {
            _UITextureImage = uiTextureImage;
            const Vec2& s = _texture2D->getContentSizeInPixels();
            VolatileTextureMgr::addDataTexture(_texture2D, uiTextureImage->getData(), s.width * s.height * 4,
                                               backend::PixelFormat::RGBA8, s);
        }
        else
        {
            AXLOGW("Cache rendertexture failed!");
        }
    };
    auto callback = std::bind(func, std::placeholders::_1);
    newImage(callback, false);
#endif
}

The call newImage(callback, false); creates the image, and the second parameter is named flipImage, but that parameter is not used in the newImage method at all. In the previous Cocos2d-x implementation, it was used, but it was always called with false anyway, and I'm not sure why.

@halx99 halx99 added the bug Something isn't working label Sep 18, 2024
@halx99
Copy link
Collaborator

halx99 commented Sep 18, 2024

The readPixels was refactor 4 years ago, I forget why, but maybe we can add a parameter to specify whether flipImage, currently backend always return a flipImage data

@rh101
Copy link
Contributor Author

rh101 commented Sep 18, 2024

Yes I see, and given RenderTexture textures are always flipped by Y by default, we need a way to flip it back after reading it from the backend.

The cocos2d-x version of Texture2DGL::getBytes has that flipImage flag, but it doesn't seem like it was ever set to true by the calls from RenderTexture::newImage.

@rh101
Copy link
Contributor Author

rh101 commented Sep 18, 2024

Flipping the image contents may be a slow process, and which gets worse with larger image size, so would any flipping happening at the time the image is being saved to the cache cause issues with when the Android app is about to be put into the background? The reason is because of Activity.onPause(), and this specific section:

onPause() execution is very brief and does not necessarily offer enough time to perform save operations. For this reason, don't use onPause() to save application or user data, make network calls, or execute database transactions. Such work might not complete before the method completes.

Instead, perform heavy-load shutdown operations during onStop(). For more information about suitable operations to perform during onStop(), see the next section. For more information about saving data, see the section about saving and restoring state.

We use onPause to read the texture data and save it to the cache, so while it is working at the moment, if things get any slower it may become an issue. The fact that the GLSurfaceView lifecycle is used for the background and foreground events is probably more of the issue. I feel that it should be refactored, so that background and foreground events are sent from the Activity.onStop() and Activity.onResume(); the GLSurfaceView lifecycle would no longer be in control of those events. We can then control when the GLSurfaceView is paused and resumed, and prevent any issues arising from an EGL context loss on the GLSurfaceView.onPause(), since we only call this when we have read and processed all the data we need from the render target.

It would be like this:

  1. Application is moved to background
  2. Activity.onPause() is called by the OS. We can't do any intensive operations in here.
  3. Activity.onStop() is called by the OS. Event EVENT_COME_TO_BACKGROUND is broadcast, which triggers the backup of texture data into the volatile texture cache, along with allowing the app to save any game data (as recommended in the onStop() documentation).
  4. Once the above is complete, we finally call GLSurfaceView.onPause(), at which point the OS may or may not invalidate the EGL context (we can't control this).
  5. When the application is brought back to the foreground, Activity.onResume() is called by the OS.
  6. We call GLSurfaceView.onResume()
  7. Broadcast EVENT_COME_TO_FOREGROUND so that all user data and cached textures are reloaded

If this is done, then we can do heavy processing when reading the texture data, meaning flipping etc., without any issues when the application is being put into the background. If we don't want to change it to the above, then perhaps we should do the flipping on recovering images from the volatile texture cache, since there are no time restrictions at this point. We would have to store a flag in the texture cache entry to indicate whether the image needs to be flipped or not.

halx99 added a commit that referenced this issue Sep 19, 2024
@halx99 halx99 linked a pull request Sep 19, 2024 that will close this issue
6 tasks
@halx99
Copy link
Collaborator

halx99 commented Sep 19, 2024

The readPixels was refactor 4 years ago, I forget why, but maybe we can add a parameter to specify whether flipImage, currently backend always return a flipImage data

I have remember it, and I make a PR for fix above 2 issues: #2166

@halx99 halx99 added this to the 2.2.0 milestone Sep 19, 2024
halx99 added a commit that referenced this issue Sep 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants