Only apply mip-maps if both image dimensions are > 512. #2409
Conversation
This is a hack / workaround for some test failures in gecko. There are some test images with resolution 2048x32. These were having mips enabled, but then failing reftests since the tests were expecting bilinear filtering only. When these images were being drawn with a size of 4x4 pixels, the smallest mip was being selected, which is not what the test expects. This workaround is a reasonable tradeoff for now - we get mip mapping on large images, while allowing the existing tests to pass. In the future, we could consider some alternatives (such as a better heuristic for enabling mips, or a different mip algorithm).
|
r? @staktrace (This should fix the test issue - just building it locally in Gecko to confirm). |
|
Confirmed that the test in question passes in gecko with this patch. |
|
In this case, with a 1/128 x 1/2 scale, ideally you'd want horizontal mip mapping but no vertical mip mapping. |
|
@mstange Yup - the initial implementation just allows the driver to build the mips. We can certainly add manual mip creation in the future though. |
|
Try push with this PR looks good, the R3 failure is gone. I'll fuzz the R8. https://treeherder.mozilla.org/#/jobs?repo=try&revision=2425a4a7e5f6ba268c41c5eee3eca310ce603d5a r+ from me, but I'm not qualified to actually review the code change. |
|
@bors-servo r+ |
|
📌 Commit 6763e44 has been approved by |
Only apply mip-maps if both image dimensions are > 512. This is a hack / workaround for some test failures in gecko. There are some test images with resolution 2048x32. These were having mips enabled, but then failing reftests since the tests were expecting bilinear filtering only. When these images were being drawn with a size of 4x4 pixels, the smallest mip was being selected, which is not what the test expects. This workaround is a reasonable tradeoff for now - we get mip mapping on large images, while allowing the existing tests to pass. In the future, we could consider some alternatives (such as a better heuristic for enabling mips, or a different mip algorithm). <!-- 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/2409) <!-- Reviewable:end -->
|
☀️ Test successful - status-appveyor, status-taskcluster, status-travis |
This is a hack / workaround for some test failures in gecko.
There are some test images with resolution 2048x32. These were
having mips enabled, but then failing reftests since the tests
were expecting bilinear filtering only. When these images were
being drawn with a size of 4x4 pixels, the smallest mip was
being selected, which is not what the test expects.
This workaround is a reasonable tradeoff for now - we get mip
mapping on large images, while allowing the existing tests to
pass.
In the future, we could consider some alternatives (such as a better
heuristic for enabling mips, or a different mip algorithm).
This change is