Bug 172485 - Snapshotting via -renderInContext: should do synchronous image decodes
Summary: Snapshotting via -renderInContext: should do synchronous image decodes
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Simon Fraser (smfr)
URL:
Keywords: InRadar
Depends on:
Blocks: 172535
  Show dependency treegraph
 
Reported: 2017-05-22 19:45 PDT by Simon Fraser (smfr)
Modified: 2023-05-23 16:28 PDT (History)
4 users (show)

See Also:


Attachments
Patch (82.33 KB, patch)
2017-05-22 20:05 PDT, Simon Fraser (smfr)
no flags Details | Formatted Diff | Diff
Patch (83.87 KB, patch)
2017-05-22 23:31 PDT, Simon Fraser (smfr)
no flags Details | Formatted Diff | Diff
Patch (86.52 KB, patch)
2017-05-23 08:18 PDT, Simon Fraser (smfr)
no flags Details | Formatted Diff | Diff
Patch (86.52 KB, patch)
2017-05-23 11:27 PDT, Simon Fraser (smfr)
no flags Details | Formatted Diff | Diff
Patch (86.53 KB, patch)
2017-05-23 11:40 PDT, Simon Fraser (smfr)
thorton: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Simon Fraser (smfr) 2017-05-22 19:45:21 PDT
Snapshotting via -renderInContext: should do synchronous image decodes
Comment 1 Simon Fraser (smfr) 2017-05-22 20:05:55 PDT
Created attachment 310974 [details]
Patch
Comment 2 Simon Fraser (smfr) 2017-05-22 20:06:19 PDT
rdar://problem/32276146
Comment 3 Build Bot 2017-05-22 20:08:51 PDT
Attachment 310974 [details] did not pass style-queue:


ERROR: Source/WebCore/platform/ios/LegacyTileCache.mm:513:  Multi line control clauses should use braces.  [whitespace/braces] [4]
ERROR: Source/WebKit/mac/WebView/WebFrame.mm:639:  Multi line control clauses should use braces.  [whitespace/braces] [4]
Total errors found: 2 in 51 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 4 Simon Fraser (smfr) 2017-05-22 23:31:30 PDT
Created attachment 310983 [details]
Patch
Comment 5 Build Bot 2017-05-22 23:33:23 PDT
Attachment 310983 [details] did not pass style-queue:


ERROR: Source/WebCore/platform/ios/LegacyTileCache.mm:513:  Multi line control clauses should use braces.  [whitespace/braces] [4]
ERROR: Source/WebKit/mac/WebView/WebFrame.mm:639:  Multi line control clauses should use braces.  [whitespace/braces] [4]
ERROR: Source/WebCore/platform/graphics/mac/WebLayer.mm:53:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Source/WebCore/platform/graphics/mac/WebLayer.mm:74:  Should not have spaces around = in property synthesis.  [whitespace/property] [4]
ERROR: Source/WebCore/platform/ios/LegacyTileLayer.mm:79:  Should not have spaces around = in property synthesis.  [whitespace/property] [4]
Total errors found: 5 in 53 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 6 Tim Horton 2017-05-22 23:36:05 PDT
Comment on attachment 310983 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=310983&action=review

> Source/WebCore/platform/graphics/ca/GraphicsLayerCA.cpp:1566
> -void GraphicsLayerCA::platformCALayerPaintContents(PlatformCALayer*, GraphicsContext& context, const FloatRect& clip)
> +void GraphicsLayerCA::platformCALayerPaintContents(PlatformCALayer*, GraphicsContext& context, const FloatRect& clip, GraphicsLayerPaintFlags flags)

Hmm, why is this not just a bit on GraphicsContext?

> Source/WebCore/platform/graphics/mac/WebLayer.mm:55
> +@interface WebSimpleLayer () {
> +    BOOL _isRenderingInContext;
> +}

Don't think you can do this in 32 bit (you'll have to put it in the real interface).
Comment 7 Simon Fraser (smfr) 2017-05-23 07:56:46 PDT
(In reply to Tim Horton from comment #6)
> Comment on attachment 310983 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=310983&action=review
> 
> > Source/WebCore/platform/graphics/ca/GraphicsLayerCA.cpp:1566
> > -void GraphicsLayerCA::platformCALayerPaintContents(PlatformCALayer*, GraphicsContext& context, const FloatRect& clip)
> > +void GraphicsLayerCA::platformCALayerPaintContents(PlatformCALayer*, GraphicsContext& context, const FloatRect& clip, GraphicsLayerPaintFlags flags)
> 
> Hmm, why is this not just a bit on GraphicsContext?

Because with filters etc you can use several different GraphicsContexts, and you'd have to propagate the bit between them. Also I think this concept is slightly higher-level than GraphicsContext.
Comment 8 Simon Fraser (smfr) 2017-05-23 08:18:23 PDT
Created attachment 311012 [details]
Patch
Comment 9 Build Bot 2017-05-23 08:20:22 PDT
Attachment 311012 [details] did not pass style-queue:


ERROR: Source/WebCore/platform/ios/LegacyTileCache.mm:513:  Multi line control clauses should use braces.  [whitespace/braces] [4]
ERROR: Source/WebKit/mac/WebView/WebFrame.mm:639:  Multi line control clauses should use braces.  [whitespace/braces] [4]
ERROR: Source/WebCore/platform/graphics/mac/WebLayer.mm:68:  Should not have spaces around = in property synthesis.  [whitespace/property] [4]
ERROR: Source/WebCore/platform/ios/LegacyTileLayer.mm:75:  Should not have spaces around = in property synthesis.  [whitespace/property] [4]
Total errors found: 4 in 57 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 10 Simon Fraser (smfr) 2017-05-23 11:27:42 PDT
Created attachment 311031 [details]
Patch
Comment 11 Build Bot 2017-05-23 11:30:40 PDT
Attachment 311031 [details] did not pass style-queue:


ERROR: Source/WebCore/platform/ios/LegacyTileCache.mm:513:  Multi line control clauses should use braces.  [whitespace/braces] [4]
ERROR: Source/WebKit/mac/WebView/WebFrame.mm:639:  Multi line control clauses should use braces.  [whitespace/braces] [4]
ERROR: Source/WebCore/platform/graphics/mac/WebLayer.mm:68:  Should not have spaces around = in property synthesis.  [whitespace/property] [4]
ERROR: Source/WebCore/platform/ios/LegacyTileLayer.mm:75:  Should not have spaces around = in property synthesis.  [whitespace/property] [4]
Total errors found: 4 in 57 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 12 Simon Fraser (smfr) 2017-05-23 11:40:01 PDT
Created attachment 311032 [details]
Patch
Comment 13 Build Bot 2017-05-23 11:41:55 PDT
Attachment 311032 [details] did not pass style-queue:


ERROR: Source/WebCore/platform/ios/LegacyTileCache.mm:513:  Multi line control clauses should use braces.  [whitespace/braces] [4]
ERROR: Source/WebKit/mac/WebView/WebFrame.mm:639:  Multi line control clauses should use braces.  [whitespace/braces] [4]
ERROR: Source/WebCore/platform/graphics/mac/WebLayer.mm:68:  Should not have spaces around = in property synthesis.  [whitespace/property] [4]
ERROR: Source/WebCore/platform/ios/LegacyTileLayer.mm:75:  Should not have spaces around = in property synthesis.  [whitespace/property] [4]
Total errors found: 4 in 57 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 14 Tim Horton 2017-05-23 12:58:28 PDT
Comment on attachment 311032 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=311032&action=review

> Source/WebCore/platform/graphics/mac/WebLayer.h:33
> +    BOOL m_isRenderingInContext;

Shouldn't have an m prefix.

> Source/WebCore/platform/graphics/mac/WebLayer.h:35
> +@property (nonatomic) BOOL _isRenderingInContext;

Why is this readwrite? Why does this have a leading underscore? This is an internal class. Should check with the SPI guidelines but I don't think the _ is required.

> Source/WebCore/platform/ios/wak/WAKWindow.h:136
> +- (void)setIsInSnapshottingPaint:(BOOL)isInSnapshottingPaint;
> +- (BOOL)isInSnapshottingPaint;

Why is this not a property :P

> Source/WebKit/mac/WebView/WebViewPrivate.h:-771
> -- (void)_setIncludesFlattenedCompositingLayersWhenDrawingToBitmap:(BOOL)flag;
> -- (BOOL)_includesFlattenedCompositingLayersWhenDrawingToBitmap;

Are you sure it's OK to remove these?
Comment 15 Simon Fraser (smfr) 2017-05-23 13:44:34 PDT
https://trac.webkit.org/r217296