New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 802924 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jan 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Bug-Regression



Sign in to add a comment

Trilinear filtering broken

Project Member Reported by reve...@chromium.org, Jan 17 2018

Issue description

This has been broken since crrev.com/9cc56b283e51de94cc373a77214da2bb646efcbd

We have a unit test for this but it's unfortunately disabled. That's because our ancient version of mesa, that used for pixel tests, doesn't support mipmaps properly. You can still run this test, but it requires real GPU usage in tests:

viz_unittests --gtest_also_run_disabled_tests --use-gpu-in-tests --gtest_filter=GLRendererPixelTest.DISABLED_TrilinearFiltering

This test produces correct results prior to above change but not after.

Note that results may not match the reference image perfectly when using real GPU but it should be a close match. There should be a thin red line in the middle of the result.

Afaict, the breakage is from GLRenderer::GenerateMipmap() being made into a noop by the CL.
 

Comment 1 by danakj@chromium.org, Jan 17 2018

We should add some unit tests that don't need to be disabled for this?
Ideally we'd uprev mesa in third_party so we can enable that pixel test. I'm not sure that's something we can realistically do as it will likely break a huge amount of layout tests and likely take a while to get building on all platforms.

Maybe we can add some GLES2Interface mock tests to verify that GenerateMipmap + TexParameteri(GL_TEXTURE_MIN_FILTER, GL_LINEAR_MIPMAP_LINEAR) are called.

Comment 3 by danakj@chromium.org, Jan 18 2018

That sounds nice, thanks
Labels: -ReleaseBlock-Dev ReleaseBlock-Beta
If this has been broken since December, it is already live on dev and too late to block dev releases on. If there is still a belief that this should block dev please let me know, but I am assuming we can block at beta.
Project Member

Comment 5 by bugdroid1@chromium.org, Jan 19 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/bb62c05b0122953851fb4b0bb4b46734b8e537d0

commit bb62c05b0122953851fb4b0bb4b46734b8e537d0
Author: Xu Xing <xing.xu@intel.com>
Date: Fri Jan 19 03:04:05 2018

[viz] Apply mipmap filter on local resource

TODO(crbug.com/803286): npot texture always return false on ubuntu desktop.
The npot texture check is probably failing on desktop GL.


BUG= 802924 , 803286

Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.android:android_optional_gpu_tests_rel
Change-Id: I8f2dc179f065f1fa59650184c59d6184e293fbfd
Reviewed-on: https://chromium-review.googlesource.com/869952
Reviewed-by: David Reveman <reveman@chromium.org>
Commit-Queue: Xing Xu <xing.xu@intel.com>
Cr-Commit-Position: refs/heads/master@{#530409}
[modify] https://crrev.com/bb62c05b0122953851fb4b0bb4b46734b8e537d0/cc/test/test_web_graphics_context_3d.h
[modify] https://crrev.com/bb62c05b0122953851fb4b0bb4b46734b8e537d0/components/viz/service/display/gl_renderer.cc
[modify] https://crrev.com/bb62c05b0122953851fb4b0bb4b46734b8e537d0/components/viz/service/display/gl_renderer_unittest.cc
[modify] https://crrev.com/bb62c05b0122953851fb4b0bb4b46734b8e537d0/components/viz/service/display/scoped_render_pass_texture.cc
[modify] https://crrev.com/bb62c05b0122953851fb4b0bb4b46734b8e537d0/components/viz/service/display/scoped_render_pass_texture.h

Components: OS>Kernel>Graphics

Comment 7 by xing...@intel.com, Jan 22 2018

Status: Fixed (was: Assigned)
Labels: Merge-TBD
[Auto-generated comment by a script] We noticed that this issue is targeted for M-65; it appears the fix may have landed after branch point, meaning a merge might be required. Please confirm if a merge is required here - if so add Merge-Request-65 label, otherwise remove Merge-TBD label. Thanks.
Project Member

Comment 9 by sheriffbot@chromium.org, Mar 6 2018

Labels: -Merge-TBD

Sign in to add a comment