Animometer - CSS bouncing blend circles renders incorrectly with GPU raster |
|||||||
Issue descriptionVersion: r395672 OS: OS X What steps will reproduce the problem? (1) https://trac.webkit.org/export/HEAD/trunk/PerformanceTests/Animometer/developer.html (2) Run "CSS bouncing blend circles" in the HTML Suite. I'm running on a MBP 15" Mid 2015 with AMD Radeon R9 M370X. It also reproduces with the Intel GPU.
,
Jun 8 2016
I can repro on linux as well.
,
Jun 8 2016
I grabbed a skp and it does not reproduce in standalone skia.
,
Jun 8 2016
There are few things going on here:
1) Chrome isn't exposing support for GL_[KHR|NV]_blend_equation_advanced even though the driver on my system supports it. This means that for complicated blends Skia will fallback to making a copy of the destination to feed into the fragment shader.
2) Skia marks the temporary surface created for the copy of the destination as having an different y-orientation than the original destination FBO.
3) When the deferred copy is actually executed Skia finds it can't come up with a way of making a copy of the destination using any of its techniques: (The "src" of the copy is the original destination of the draw and the "dst" of the copy is the temporary dst copy).
a) We can't do a draw because Skia knows the src's FBO ID but doesn't have a texture ID for it
b) We can't do a glCopyTexSubImage2D because the src and dst are BGRA and this isn't spec'ed for
GL_EXT_texture_format_BGRA8888.
c) We can't do a glBlitFramebuffer because GL_CHROMIUM_framebuffer_multisample doesn't allow mirroring
blits and we chose a different y orientation for the dst in 2).
I have a change that fixes this by making the temporary in 2) use the same orientation as the original dst. This allows us to do a glBlitFramebuffer to make the copy.
However, it seems like we should also address 1) so that we don't need to make a copy for shader based blending as well. Alternatively, if GL_NV_texture_barrier were exposed Skia would use that to perform shader-based blending without a copy.
,
Jun 8 2016
The following revision refers to this bug: https://skia.googlesource.com/skia.git/+/5c7d62d431a0ad7988f05fbdb206237729c0c308 commit 5c7d62d431a0ad7988f05fbdb206237729c0c308 Author: bsalomon <bsalomon@google.com> Date: Wed Jun 08 17:02:42 2016 When setting up a copySurface dst texture make the orientation match the src when glBlitFramebuffer requires it BUG= chromium:618122 GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2049753002 Review-Url: https://codereview.chromium.org/2049753002 [modify] https://crrev.com/5c7d62d431a0ad7988f05fbdb206237729c0c308/src/gpu/gl/GrGLGpu.cpp
,
Jun 8 2016
Thanks for figuring this out! I'll file bugs about exposing these extensions. I think we should merge the fix into M52, as this bug will cause visual artifacts on sites that use "mix-blend-mode".
,
Jun 8 2016
,
Jun 8 2016
Your change meets the bar and is auto-approved for M52 (branch: 2743)
,
Jun 8 2016
The following revision refers to this bug: https://skia.googlesource.com/skia.git/+/5bfd891d97fc4ac0b54aef0062474018d0e73389 commit 5bfd891d97fc4ac0b54aef0062474018d0e73389 Author: bsalomon <bsalomon@google.com> Date: Wed Jun 08 19:32:02 2016 When setting up a copySurface dst texture make the orientation match the src when glBlitFramebuffer requires it TBR=egdaniel@google.com BUG= chromium:618122 GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2049753002 Review-Url: https://codereview.chromium.org/2049753002 Cherry-pick from master to chrome/m52 NOTREECHECKS=true NOTRY=true NOPRESUBMIT=true Review-Url: https://codereview.chromium.org/2049493003 [modify] https://crrev.com/5bfd891d97fc4ac0b54aef0062474018d0e73389/src/gpu/gl/GrGLGpu.cpp
,
Jun 8 2016
,
Jun 8 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/d214d26887a2d522c80ef35b54d09cc242aac8e2 commit d214d26887a2d522c80ef35b54d09cc242aac8e2 Author: skia-deps-roller <skia-deps-roller@chromium.org> Date: Wed Jun 08 20:34:57 2016 Roll src/third_party/skia/ 7942f22c6..2a437e645 (4 commits). https://chromium.googlesource.com/skia.git/+log/7942f22c607c..2a437e64593a $ git log 7942f22c6..2a437e645 --date=short --no-merges --format='%ad %ae %s' 2016-06-08 liyuqian Provide filter when there are many options 2016-06-08 csmartdalton Replace targetHasUnifiedMultisampling in GrPB constructor 2016-06-08 bsalomon When setting up a copySurface dst texture make the orientation match the src when glBlitFramebuffer requires it 2016-06-08 msarett Modify QCMS test to match use by Chrome BUG= 618122 CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel TBR=mtklein@google.com Review-Url: https://codereview.chromium.org/2044293003 Cr-Commit-Position: refs/heads/master@{#398662} [modify] https://crrev.com/d214d26887a2d522c80ef35b54d09cc242aac8e2/DEPS
,
Jun 8 2016
Confirmed with Dev, the change was in Skia. It was cherry picked it to the M52 branch of Skia. So, the next build of M52 Chrome will pick it up. Thank you Brian for merging it to the M52 branch quickly.
,
Jun 12 2016
This issue has been approved for a merge. Please merge the fix to any appropriate branches as soon as possible! If all merges have been completed, please remove any remaining Merge-Approved labels from this issue. Thanks for your time! To disable nags, add the Disable-Nags label. For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Jun 13 2016
|
|||||||
►
Sign in to add a comment |
|||||||
Comment 1 by vmi...@chromium.org
, Jun 8 2016