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

Should roborazzi use hardware mode? #296

Open
yschimke opened this issue Apr 11, 2024 · 16 comments
Open

Should roborazzi use hardware mode? #296

yschimke opened this issue Apr 11, 2024 · 16 comments

Comments

@yschimke
Copy link
Contributor

Possibly related to #290

I had some regressions switching off my own homegrown screenshot code to Roborazzi.

Is roborazzi enabling hardware mode in all cases? Is setting USE_HARDWARE_RENDERER_NATIVE_ENV enough? Do we need to wrap tests with something like

        public fun <R> withDrawingEnabled(block: () -> R): R {
            val wasDrawingEnabled = HardwareRendererCompat.isDrawingEnabled()
            try {
                if (!wasDrawingEnabled) {
                    HardwareRendererCompat.setDrawingEnabled(true)
                }
                return block.invoke()
            } finally {
                if (!wasDrawingEnabled) {
                    HardwareRendererCompat.setDrawingEnabled(false)
                }
            }
        }
@takahirom
Copy link
Owner

I'm actually not sure if we should recommend using it since it's not mentioned in Robolectric's release notes. @utzcoz, sorry for mentioning you suddenly, but do you have any information about this?

@yschimke
Copy link
Contributor Author

A related discussion robolectric/robolectric#8592

I think from that I thought it was released in 4.12

@utzcoz
Copy link

utzcoz commented Apr 11, 2024

@takahirom No worry, you can ping me anytime when you need.

I'm actually not sure if we should recommend using it since it's not mentioned in Robolectric's release notes.

I think from that I thought it was released in 4.12

I think we can use hardware mode on macOS, Windows and Linux with Robolectric 4.12+. We can see many related improvements for recent releases:

https://github.com/search?q=repo%3Arobolectric%2Frobolectric+HardwareRenderer&type=pullrequests
https://android-review.googlesource.com/c/platform/frameworks/base/+/2887086
https://android-review.googlesource.com/c/platform/frameworks/base/+/2874866

But we also find some hardware mode related issues, for example, RenderEffect Blur related issue: robolectric/robolectric#8955. Also there are some limitations of current RNG: robolectric/robolectric#8955 (comment). Maybe the next Android version supporting will improve them.

As I am not th person to participate in the RNG project mainly, so I only can provide my suggestions:

  1. It's recommended to try it.
  2. But you need to have expectations that there are some fidelity issues. Also we need to know there are some features don't behave the same on the different OSes.

So if you only use one OS to run screenshot testing, like Linux or macOS, or Linux and macOS, you can try to enable it for current test suite and check its behavior. For low-fidelity scenarios, feel free to open issues to Robolectric. So it depends on.

Let's ping @hoisie friendly for more professional suggestions. And @hoisie, maybe we can record it to our FAQ?

@yschimke
Copy link
Contributor Author

@utzcoz is the system property (robolectric.screenshot.hwrdr.native) the only thing needed? That appears to be what chrisbanes/haze#147 is doing.

Am I setting it incorrectly, or does HardwareRendererCompat.setDrawingEnabled do something subtly different?

@sergio-sastre
Copy link
Contributor

@utzcoz is the system property (robolectric.screenshot.hwrdr.native) the only thing needed? That appears to be what chrisbanes/haze#147 is doing.

Am I setting it incorrectly, or does HardwareRendererCompat.setDrawingEnabled do something subtly different?

It only works on API 31+

I’ve updated Android-Screenshot Testing Playground to use hardware native (activated in the gradle file) in case it helps you

@utzcoz
Copy link

utzcoz commented May 7, 2024

@sergio-sastre Could you help send a PR for Robolectric to add necessary documentation for API requirement?

@hoisie
Copy link

hoisie commented May 7, 2024

I just wanted to let everyone know that we have made some significant bug fixes to HW rendering in the past couple of weeks, and are planning to do a new Robolectric point release (4.12.2) with these fixes.

@yschimke
Copy link
Contributor Author

yschimke commented May 7, 2024

@hoisie @utzcoz if it ever feels like complaints and negativity, please know it's amazing to see how fast it's improving, and how quickly these features are getting adopted, and solving real problems.

@sergio-sastre
Copy link
Contributor

sergio-sastre commented May 11, 2024

@sergio-sastre Could you help send a PR for Robolectric to add necessary documentation for API requirement?

Sure I can help, but I am just unsure where should I add it? Should I create a new section in the README.md?

@utzcoz
Copy link

utzcoz commented May 11, 2024

@sergio-sastre Maybe robolectric.github.io.

@takahirom
Copy link
Owner

takahirom commented May 18, 2024

Robolectric 4.12.2 is out and I shared that the flag's name has been changed here, as @utzcoz informed me. Thank you for providing this information.
robolectric.screenshot.hwrdr.native=true

robolectric.pixelCopyRenderMode=hardware

@takahirom
Copy link
Owner

Surprisingly, without the robolectric.pixelCopyRenderMode option, even basic column layouts and some components seem to be affected. It might be best to enable this option when using Roborazzi.
#408

@hoisie
Copy link

hoisie commented Jun 27, 2024

@takahirom if basic column layouts are not working in SW rendering mode, it may indicate a bug in Compose.

I know that the Compose team has deprioritized SW rendering, but I do believe that simple layouts should work.

@adrinieto-jt
Copy link

We enabled hardware mode in our project, and everything looks good so far, except when using captureScreenRoboImage() to capture screens with dialogs, the scrim is not captured. 🤷

This is what I see in the Compose Preview and what I expected to capture:

image

But this is what it's captured. As you can see the shadow of the card is there, but not the scrim. Not sure if this is related to the hardware mode or the way captureScreenRoboImage() works.

image

@takahirom
Copy link
Owner

@adrinieto-jt
Sorry for being late. I think the issue where the window background isn't used is a different issue from hardware mode. We are merging window images, but the decorView doesn't have the background.

rootsOrderByDepth.forEach { root ->
val layoutParams = root.windowLayoutParams.get()
val decorView = root.decorView
val outRect = Rect()
Gravity.apply(
layoutParams.gravity,
root.decorView.width,
root.decorView.height,
Rect(0, 0, screenDecorView.width, screenDecorView.height),
layoutParams.x,
layoutParams.y,
outRect
)
decorView.fetchImage(
recordOptions = roborazziOptions.recordOptions,
)?.let {
canvas.drawBitmap(it, outRect.left.toFloat(), outRect.top.toFloat(), null)
}
}

@takahirom
Copy link
Owner

I noticed that there is a commit indicating that the hardware mode will be set as the default option.
robolectric/robolectric@17437ec

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants