Bug 149424

Summary: CSSFilterImageValue::image makes an unconditionally unaccelerated ImageBuffer
Product: WebKit Reporter: Tim Horton <thorton>
Component: CSSAssignee: Nobody <webkit-unassigned>
Status: NEW    
Severity: Normal CC: dino, esprehn+autocc, ews-watchlist, frankhome61, glenn, gyuyoung.kim, macpherson, menard, sabouhallawa, simon.fraser
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=235376
Attachments:
Description Flags
Patch none

Tim Horton
Reported 2015-09-21 15:40:32 PDT
The ImageBuffer should inherit the accelerated bit from the context it will eventually be painted into.
Attachments
Patch (1.91 KB, patch)
2020-09-03 11:22 PDT, frankhome61 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
frankhome61
Comment 1 2020-09-03 11:22:03 PDT
Created attachment 407897 [details] Patch
frankhome61
Comment 2 2020-09-03 11:23:43 PDT
Should just consult renderer.page().settings() to get the rendering mode setting.
Simon Fraser (smfr)
Comment 3 2020-09-03 11:39:42 PDT
Comment on attachment 407897 [details] Patch This seems wrong. It should only make an accelerated buffer if we know the filter chain can be rendered via Core Image.
frankhome61
Comment 4 2020-09-03 11:44:03 PDT
(In reply to Simon Fraser (smfr) from comment #3) > Comment on attachment 407897 [details] > Patch > > This seems wrong. It should only make an accelerated buffer if we know the > filter chain can be rendered via Core Image. Oh this has nothing to do with CoreImage, this is simply trying to address the fact that the ImageBuffer created inside CSSFilterImageValue is Unaccelerated unconditionally, and pass it on to CSSFilter. CoreImage is not involved in this process.
frankhome61
Comment 5 2020-09-03 11:45:19 PDT
(In reply to guowei_yang from comment #4) > (In reply to Simon Fraser (smfr) from comment #3) > > Comment on attachment 407897 [details] > > Patch > > > > This seems wrong. It should only make an accelerated buffer if we know the > > filter chain can be rendered via Core Image. > > Oh this has nothing to do with CoreImage, this is simply trying to address > the fact that the ImageBuffer created inside CSSFilterImageValue is > Unaccelerated unconditionally, and pass it on to CSSFilter. CoreImage is not > involved in this process. P.S. this patch has nothing to do with my project, just something I spotted while looking through different files
frankhome61
Comment 6 2020-09-03 11:47:18 PDT
(In reply to Simon Fraser (smfr) from comment #3) > Comment on attachment 407897 [details] > Patch > > This seems wrong. It should only make an accelerated buffer if we know the > filter chain can be rendered via Core Image. Also before we introduced CoreImage into the codebase, CSSFilter::allocateBackingStoreIfNeeded() also consults renderer.page().settings() to determine which kind of ImageBuffer backend to create. So I would only assume the same procedure should happen here.
Simon Fraser (smfr)
Comment 7 2020-09-03 11:55:51 PDT
settings().acceleratedFiltersEnabled() was added for another platform and used to do nothing on Apple platforms. You've started to use it for the CI acceleration, which is fine. However, we know this code path (on Apple platforms) always hits the non-accelerated filter path, so we should not use this switch to make an accelerated buffer; that will slow things down.
frankhome61
Comment 8 2020-09-03 11:57:24 PDT
(In reply to Simon Fraser (smfr) from comment #7) > settings().acceleratedFiltersEnabled() was added for another platform and > used to do nothing on Apple platforms. You've started to use it for the CI > acceleration, which is fine. > > However, we know this code path (on Apple platforms) always hits the > non-accelerated filter path, so we should not use this switch to make an > accelerated buffer; that will slow things down. Oh okay. I guess this bug should be ignored then
frankhome61
Comment 9 2020-09-03 11:57:34 PDT
(In reply to guowei_yang from comment #8) > (In reply to Simon Fraser (smfr) from comment #7) > > settings().acceleratedFiltersEnabled() was added for another platform and > > used to do nothing on Apple platforms. You've started to use it for the CI > > acceleration, which is fine. > > > > However, we know this code path (on Apple platforms) always hits the > > non-accelerated filter path, so we should not use this switch to make an > > accelerated buffer; that will slow things down. > > Oh okay. I guess this bug should be ignored then "bug"
Note You need to log in before you can comment on or make changes to this bug.