Re-enable DRAWWINDOW_DRAW_VIEW flag for Marionette screen captures
Categories
(Remote Protocol :: Marionette, enhancement, P3)
Tracking
(Not tracked)
People
(Reporter: ato, Unassigned)
References
Details
Attachments
(1 obsolete file)
As noted in a TODO in testing/marionette/capture.js, we have disabled two flags on the canvas context when taking screenshots:
> let ctx = canvas.getContext(CONTEXT_2D);
> if (flags === null) {
> flags = ctx.DRAWWINDOW_DRAW_CARET;
> // Disabled in bug 1243415 for webplatform-test
> // failures due to out of view elements. Needs
> // https://github.com/w3c/web-platform-tests/issues/4383 fixed.
> /*
> ctx.DRAWWINDOW_DRAW_VIEW;
> */
> // Bug 1009762 - Crash in [@ mozilla::gl::ReadPixelsIntoDataSurface]
> /*
> ctx.DRAWWINDOW_USE_WIDGET_LAYERS;
> */
> }
We want to investigate re-enabling these.
Comment 1•7 years ago
|
||
You could try to test in a try push but the underlying crash is not fixed yet, and as such it should still appear.
Updated•7 years ago
|
Comment 2•5 years ago
|
||
Note that with the patch on bug 1559592 landed this bug will become invalid.
Comment 3•5 years ago
|
||
Actually let me fix that bug first, which will help me with the refactoring on bug 1559592 afterward.
Comment 4•5 years ago
|
||
Note that both flags can be re-enabled now given that all dependencies have been fixed, or are no longer reproducible.
Comment 5•5 years ago
|
||
Comment 6•5 years ago
|
||
All dependencies are fixed, or no longer reproducible. As such both
flags can be re-enabled. Also reftests won't have to specify the
exactly same flags on its own anymore.
Pushed by hskupin@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/4dab6e3dc6ed [marionette] Re-enable DRAWWINDOW_DRAW_VIEW and DRAWWINDOW_USE_WIDGET_LAYERS flags for screen captures. r=webdriver-reviewers,ato
Comment 8•5 years ago
|
||
bugherder |
Comment 9•5 years ago
|
||
I would suggest to backout this patch given the fact that doing full screenshots are not working with the DRAWWINDOW_DRAW_VIEW
flag not being present. There is a strange behavior change in combination with DRAWWINDOW_USE_WIDGET_LAYERS
maybe. And given that we shortly change the API to drawSnapshot()
we could try to get it working properly there again.
Comment 10•5 years ago
|
||
Comment 11•5 years ago
|
||
We will have to wait for bug 1571341 before we can try to enable the flags again.
Comment 12•5 years ago
|
||
The new API only got the DRAWWINDOW_DRAW_VIEW
flag. Using the widget one isn't necessary because drawSnapshot
doesn't readback the data from the compositor.
Given that bug 1559592 hasn't been finished yet, and I don't want to cause a regression with its landing, I will most likely have to take care of it now. So lets mark this bug dependent on it instead.
Comment 13•5 years ago
|
||
Actually by moving to the new snapshot API we no longer need that flag. When only the viewport has to be captured the correct absolute coordinates will have to be specified including the x and y scroll position. It's way more flexible now.
Updated•3 years ago
|
Updated•1 year ago
|
Description
•