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

Random glitch when capturing screenshot #964

Open
ri0ter opened this issue Jan 20, 2017 · 7 comments
Open

Random glitch when capturing screenshot #964

ri0ter opened this issue Jan 20, 2017 · 7 comments

Comments

@ri0ter
Copy link

ri0ter commented Jan 20, 2017

Sometimes when using screenshot method in top left corner a small dot is visible on captured image.
After a short research I found out that a call to DOM.highlightRect command here causes the problem.

@rosshinkley
Copy link
Contributor

This is (kind of) by design - using highlightRect as opposed to touching the DOM directly solves a lot more problems than it creates (see #927 and the issues referenced there).

I wonder if the HIGHLIGHT_STYLE could be determined at runtime to mimic the top left pixel, or if this is a case where the highlighted pixel isn't fully rendered at screenshot time? Might be worth some experimentation.

/cc @Mr0grog, as he is way closer to this than I am, and might have more ideas.

@Mr0grog
Copy link
Contributor

Mr0grog commented Jan 20, 2017

I wonder if the HIGHLIGHT_STYLE could be determined at runtime to mimic the top left pixel

I unfortunately can’t think of a reliable way to do this, BUT… we I think we could hold the callback until after the DOM.hideHighlight call is complete. Actually, I’m not sure why I didn’t do that to begin with.

@Mr0grog
Copy link
Contributor

Mr0grog commented Jan 20, 2017

I’m pretty sure the fix for this is the same fix as for #955 and will depend on the work being done in #945.

@ri0ter
Copy link
Author

ri0ter commented Jan 23, 2017

I'm kind new to this topic but what about using invalidate() method? As the documentation says it generates new frame and it seems to be the easiest solution. Is there anything wrong with it?
I don't know how about other cases but I did tested it with my own and it seems to work.

@Mr0grog
Copy link
Contributor

Mr0grog commented Jan 23, 2017

I'm kind new to this topic but what about using invalidate() method?

invalidate() only works when offline rendering is used (literally, it does nothing if not using offline rendering). It was explored in this changeset: 53dee8a

That approach was thrown out because it comes with a whole host of other issues and downsides (if your goal is solely to make screenshots of webpages, it’s great; otherwise it may not work well at all). There wasn’t much discussion of it, so no worries for not knowing about it.

@matthewmueller
Copy link
Contributor

matthewmueller commented Feb 21, 2018

Hey guys – do you mind catching me up on this issue a bit? It seems like the frame manager was added to address out-of-date frames.

Is that still an issue on electron's side? And do you have any example scripts I can try out that would illustrate this problem?

@Mr0grog
Copy link
Contributor

Mr0grog commented Feb 21, 2018

If it helps, here’s a version of @ri0ter’s link from the initial post that has the highlight in the correct place: https://github.com/segmentio/nightmare/blob/dbb1b6413389eb551b9bc361d66b316172f4dbb8/lib/frame-manager.js#L102-L106

The frame manager works by highlighting and unhighlighting a portion of the page (in the top left corner) to force a frame (in animation terms, not geometry terms) to be rendered, which ensures that the frame buffer in the browser process (not renderer process, which is already good) is up-to-date.

Unfortunately, the frame manager signals that its done its job after sending the command to hide the highlight, but not before waiting for a frame where the highlight is hidden. So sometimes the frame you get in the captured screenshot has the highlight, which is the pixel @ri0ter is talking about.

As noted before, the fix described in my first commment on #955 is the solution to this problem (short of fixing Electron so that calls to capture the page always have an up-to-date frame buffer, which was a hard problem a year ago—not sure if Chromium internals have changed enough to make that easier now).

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

No branches or pull requests

5 participants