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

Default OOP rasterization to off (original value) #4318

Merged
merged 1 commit into from
Jan 9, 2020

Conversation

bsclifton
Copy link
Member

@bsclifton bsclifton commented Jan 7, 2020

OOP rasterization is causing issues for some users (artifacts, crashes)

Resolves problems some users are having with crashes and graphic artifacts (blurred or wrong pixel values). Removing the flag for this would regress performance for folks not affected. We may want to look at a different (safer) way to solve brave/brave-browser#6613 (which I believe that Chrome is solving via field trials)

Deviations page doesn't need updating, since details about force enabling weren't originally added

Fixes brave/brave-browser#7581
Fixes brave/brave-browser#6979

Reverts #3793
Unfixes brave/brave-browser#6613

Submitter Checklist:

Test Plan:

  1. Visit brave://version
  2. Verify that Command Line: does not have --enable-oop-rasterization=Enabled

Reviewer Checklist:

  • New files have MPL-2.0 license header.
  • Request a security/privacy review as needed.
  • Adequate test coverage exists to prevent regressions
  • Verify test plan is specified in PR before merging to source

After-merge Checklist:

  • The associated issue milestone is set to the smallest version that the
    changes has landed on.
  • All relevant documentation has been updated.

@bsclifton bsclifton added this to the 1.5.x - Nightly milestone Jan 7, 2020
@bsclifton
Copy link
Member Author

@bbondy if this is approved/merged, should we re-open brave/brave-browser#6613?

@bsclifton bsclifton changed the title OOP rasterization is causing issues for some users (artifacts, crashes) Default OOP rasterization to off (original value) Jan 7, 2020
@bsclifton bsclifton self-assigned this Jan 7, 2020
@bsclifton
Copy link
Member Author

test-install failed on macOS, otherwise CI did great 👍 (all platforms built / tests pass)

@iefremov
Copy link
Contributor

iefremov commented Jan 9, 2020

@bsclifton Does it really fix the mentioned issues? There is some feedback saying it doesn't. I'm basically OK with merging it (since it seems it is still in trials in Chrome), just want to understand trade-offs.

@bsclifton
Copy link
Member Author

bsclifton commented Jan 9, 2020

@iefremov it does indeed fix both display glitches and specific crash situations (most which have been fixed are Linux). But it doesn't fix as many situations as I had hoped. That said, it still is a deviation from Chromium in an area which I'm personally not sure if it makes sense to differentiate

I believe there have been about 4 reports which I have been able to confirm that disabling fixed the problem (likely helps a lot of people not reporting). The tradeoff would be: for folks not experiencing crash or graphical artifacts, there would be a slight performance loss

@kjozwiak
Copy link
Member

kjozwiak commented Jan 10, 2020

Verification PASSED on macOS 10.15.2 x64 using the following build:

Brave 1.5.25 Chromium: 79.0.3945.117 (Official Build) nightly (64-bit)
Revision 04f0a055010adab4484f7497fbfdbf312c307f1d-refs/branch-heads/3945@{#1019}
OS macOS Version 10.15.2 (Build 19C57)

Screen Shot 2020-01-10 at 11 47 43 AM

Verification passed on

Brave 1.5.25 Chromium: 79.0.3945.117 (Official Build) nightly (64-bit)
Revision 04f0a055010adab4484f7497fbfdbf312c307f1d-refs/branch-heads/3945@{#1019}
OS Ubuntu 18.04 LTS

image

Verification passed on

Brave 1.5.25 Chromium: 79.0.3945.117 (Official Build) nightly (64-bit)
Revision 04f0a055010adab4484f7497fbfdbf312c307f1d-refs/branch-heads/3945@{#1019}
OS Windows 10 OS Version 1803 (Build 17134.1006)

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