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

Red flicker fix/workaround #2280

Merged
merged 1 commit into from
Jan 14, 2018
Merged

Red flicker fix/workaround #2280

merged 1 commit into from
Jan 14, 2018

Conversation

kvark
Copy link
Member

@kvark kvark commented Jan 10, 2018

Addresses https://bugzilla.mozilla.org/show_bug.cgi?id=1421696
TODO: gecko try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=27240e664f521285aa3d3ae76fce18886eba46c1&selectedJob=155374418

PR contains 3 things:

  1. extensive validation of the render target list state with assertions. Note: none of those were triggered by the bug, but it's good to have them anyway for the future.
  2. fixed reset to the previous FBO in update_texture_storage (this may fix some other issues, technically)
  3. WORK_AROUND_TEX_IMAGE fix, which resets a render target before trying to re-initialize it

This change is Reviewable

@kvark
Copy link
Member Author

kvark commented Jan 10, 2018

@glennw try appears green (minus some sporadic errors unrelated to WR)

@glennw
Copy link
Member

glennw commented Jan 10, 2018

Wow, that's an unfortunate bug :( Are there any comments you could add to that workaround bool with links to further information (I guess the bugzilla bug, or is there anything known on the angle bug list etc?).

@bors-servo r+

@bors-servo
Copy link
Contributor

📌 Commit 94250b3 has been approved by glennw

@kvark
Copy link
Member Author

kvark commented Jan 10, 2018

There is no simplified use-case yet, and no associated Angle issue. I'll make sure to correct the comment when we know more...

@bors-servo
Copy link
Contributor

⌛ Testing commit 94250b3 with merge b277f45...

bors-servo pushed a commit that referenced this pull request Jan 10, 2018
Red flicker fix/workaround

Addresses https://bugzilla.mozilla.org/show_bug.cgi?id=1421696
~~TODO:~~ gecko try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=27240e664f521285aa3d3ae76fce18886eba46c1&selectedJob=155374418

PR contains 3 things:
  1. extensive validation of the render target list state with assertions. Note: none of those were triggered by the bug, but it's good to have them anyway for the future.
  2. fixed reset to the previous FBO in `update_texture_storage` (this may fix some other issues, technically)
  3. `WORK_AROUND_TEX_IMAGE ` fix, which resets a render target before trying to re-initialize it

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/webrender/2280)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

💔 Test failed - status-travis

@kvark
Copy link
Member Author

kvark commented Jan 11, 2018

OSX Travis is having a moment...
@bors-servo retry

@bors-servo
Copy link
Contributor

⌛ Testing commit 94250b3 with merge daa05ad...

bors-servo pushed a commit that referenced this pull request Jan 11, 2018
Red flicker fix/workaround

Addresses https://bugzilla.mozilla.org/show_bug.cgi?id=1421696
~~TODO:~~ gecko try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=27240e664f521285aa3d3ae76fce18886eba46c1&selectedJob=155374418

PR contains 3 things:
  1. extensive validation of the render target list state with assertions. Note: none of those were triggered by the bug, but it's good to have them anyway for the future.
  2. fixed reset to the previous FBO in `update_texture_storage` (this may fix some other issues, technically)
  3. `WORK_AROUND_TEX_IMAGE ` fix, which resets a render target before trying to re-initialize it

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/webrender/2280)
<!-- Reviewable:end -->
@glennw
Copy link
Member

glennw commented Jan 11, 2018

@bors-servo r+ force retry

@glennw glennw closed this Jan 11, 2018
@glennw glennw reopened this Jan 11, 2018
// Apparently, in some cases calling `glTexImage3D` with
// similar parameters that the texture already has confuses
// Angle when running with optimizations.
const WORK_AROUND_TEX_IMAGE: bool = cfg!(windows);
Copy link
Contributor

@nical nical Jan 11, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bors-servo
Copy link
Contributor

💥 Test timed out

@jrmuizel
Copy link
Collaborator

@bors-servo retry

@bors-servo
Copy link
Contributor

⌛ Testing commit 94250b3 with merge 6160d3a...

bors-servo pushed a commit that referenced this pull request Jan 11, 2018
Red flicker fix/workaround

Addresses https://bugzilla.mozilla.org/show_bug.cgi?id=1421696
~~TODO:~~ gecko try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=27240e664f521285aa3d3ae76fce18886eba46c1&selectedJob=155374418

PR contains 3 things:
  1. extensive validation of the render target list state with assertions. Note: none of those were triggered by the bug, but it's good to have them anyway for the future.
  2. fixed reset to the previous FBO in `update_texture_storage` (this may fix some other issues, technically)
  3. `WORK_AROUND_TEX_IMAGE ` fix, which resets a render target before trying to re-initialize it

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/webrender/2280)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

💔 Test failed - status-travis

@glennw
Copy link
Member

glennw commented Jan 11, 2018

@bors-servo retry

@bors-servo
Copy link
Contributor

⌛ Testing commit 94250b3 with merge abd33fe...

bors-servo pushed a commit that referenced this pull request Jan 13, 2018
Red flicker fix/workaround

Addresses https://bugzilla.mozilla.org/show_bug.cgi?id=1421696
~~TODO:~~ gecko try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=27240e664f521285aa3d3ae76fce18886eba46c1&selectedJob=155374418

PR contains 3 things:
  1. extensive validation of the render target list state with assertions. Note: none of those were triggered by the bug, but it's good to have them anyway for the future.
  2. fixed reset to the previous FBO in `update_texture_storage` (this may fix some other issues, technically)
  3. `WORK_AROUND_TEX_IMAGE ` fix, which resets a render target before trying to re-initialize it

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/webrender/2280)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

💔 Test failed - status-travis

@glennw
Copy link
Member

glennw commented Jan 13, 2018

@bors-servo retry

@bors-servo
Copy link
Contributor

⌛ Testing commit 94250b3 with merge 1f900fa...

bors-servo pushed a commit that referenced this pull request Jan 13, 2018
Red flicker fix/workaround

Addresses https://bugzilla.mozilla.org/show_bug.cgi?id=1421696
~~TODO:~~ gecko try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=27240e664f521285aa3d3ae76fce18886eba46c1&selectedJob=155374418

PR contains 3 things:
  1. extensive validation of the render target list state with assertions. Note: none of those were triggered by the bug, but it's good to have them anyway for the future.
  2. fixed reset to the previous FBO in `update_texture_storage` (this may fix some other issues, technically)
  3. `WORK_AROUND_TEX_IMAGE ` fix, which resets a render target before trying to re-initialize it

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/webrender/2280)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

💔 Test failed - status-travis

@kvark
Copy link
Member Author

kvark commented Jan 14, 2018

@bors-servo retry

@bors-servo
Copy link
Contributor

⌛ Testing commit 94250b3 with merge 7b91e95...

bors-servo pushed a commit that referenced this pull request Jan 14, 2018
Red flicker fix/workaround

Addresses https://bugzilla.mozilla.org/show_bug.cgi?id=1421696
~~TODO:~~ gecko try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=27240e664f521285aa3d3ae76fce18886eba46c1&selectedJob=155374418

PR contains 3 things:
  1. extensive validation of the render target list state with assertions. Note: none of those were triggered by the bug, but it's good to have them anyway for the future.
  2. fixed reset to the previous FBO in `update_texture_storage` (this may fix some other issues, technically)
  3. `WORK_AROUND_TEX_IMAGE ` fix, which resets a render target before trying to re-initialize it

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/webrender/2280)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

☀️ Test successful - status-appveyor, status-travis
Approved by: glennw
Pushing 7b91e95 to master...

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

Successfully merging this pull request may close these issues.

5 participants