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

Share gradient GPU cache entries for repeated gradient primitives. #2454

Merged
merged 1 commit into from Feb 22, 2018

Conversation

@glennw
Copy link
Member

glennw commented Feb 22, 2018

This is a potential fix for the intermittent failures here:
#2441 (comment)

I still can't reproduce them locally. However, I could see that the
GPU cache size in that test was getting dangerously close to the
maximum size of the texture supported by the GL implementation.

That would fit with the issues we're seeing on CI, where there
seems to be items not being drawn, and they are affected even
though the test in question doesn't draw any gradients.

This is not a very elegant fix - however it will hopefully fix
the issues we are seeing on CI for now. We can revisit this
later to consider a better fix.


This change is Reviewable

This is a potential fix for the intermittent failures here:
#2441 (comment)

I still can't reproduce them locally. However, I could see that the
GPU cache size in that test was getting dangerously close to the
maximum size of the texture supported by the GL implementation.

That would fit with the issues we're seeing on CI, where there
seems to be items not being drawn, and they are affected even
though the test in question doesn't draw any gradients.

This is not a very elegant fix - however it will hopefully fix
the issues we are seeing on CI for now. We can revisit this
later to consider a better fix.
@glennw
Copy link
Member Author

glennw commented Feb 22, 2018

Try looks green with this patch @staktrace

@kvark
kvark approved these changes Feb 22, 2018
pub handle: GpuCacheHandle,
}

impl CachedGradient {

This comment has been minimized.

@kvark

kvark Feb 22, 2018

Member

we should probably just derive(Default) for cases like this

@kvark
Copy link
Member

kvark commented Feb 22, 2018

@bors-servo
Copy link
Contributor

bors-servo commented Feb 22, 2018

📌 Commit 58a1de7 has been approved by kvark

@bors-servo
Copy link
Contributor

bors-servo commented Feb 22, 2018

Testing commit 58a1de7 with merge 99e18ff...

bors-servo added a commit that referenced this pull request Feb 22, 2018
Share gradient GPU cache entries for repeated gradient primitives.

This is a potential fix for the intermittent failures here:
#2441 (comment)

I still can't reproduce them locally. However, I could see that the
GPU cache size in that test was getting dangerously close to the
maximum size of the texture supported by the GL implementation.

That would fit with the issues we're seeing on CI, where there
seems to be items not being drawn, and they are affected even
though the test in question doesn't draw any gradients.

This is not a very elegant fix - however it will hopefully fix
the issues we are seeing on CI for now. We can revisit this
later to consider a better fix.

<!-- 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/2454)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Feb 22, 2018

☀️ Test successful - status-appveyor, status-taskcluster, status-travis
Approved by: kvark
Pushing 99e18ff to master...

@bors-servo bors-servo merged commit 58a1de7 into servo:master Feb 22, 2018
3 checks passed
3 checks passed
Taskcluster (pull_request) TaskGroup: success
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
homu Test successful
Details
@staktrace
Copy link
Contributor

staktrace commented Feb 22, 2018

Fix looks good, thanks!

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

Successfully merging this pull request may close these issues.

None yet

5 participants
You can’t perform that action at this time.