Bug 177032

Summary:

Web Inspector: Canvas: recording parameters that include an Image should show an InlineSwatch (2D canvas)

Product:

WebKit

Reporter:

Devin Rousso <hi>

Component:

Web Inspector

Assignee:

Devin Rousso <hi>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

bburg, buildbot, commit-queue, inspector-bugzilla-changes, mattbaker, rniwa, webkit-bug-importer

Priority:

Keywords:

InRadar

Version:

WebKit Nightly Build

Hardware:

All

OS:

All

Bug Depends on:

Bug Blocks:

173807, 179207

Attachments:

Description	Flags
Patch	none
[Image] After Patch is applied	none
Patch	none
[Image] After Patch is applied	none
Patch	none
Archive of layout-test-results from ews103 for mac-elcapitan	none
Archive of layout-test-results from ews113 for mac-elcapitan	none
Archive of layout-test-results from ews106 for mac-elcapitan-wk2	none
Patch	none
Patch	none

Devin Rousso

Reported 2017-09-15 22:54:42 PDT

For Image: - createPattern - drawImage - drawImageFromRect For ImageData: - createImageData - putImageData - webkitPutImageDataHD For CanvasPattern: - fillStyle - strokeStyle For CanvasGradient (can also treated as an image, due to its complexity compared to a CSS color): - fillStyle - strokeStyle

Attachments
Patch (20.54 KB, patch) 2017-09-15 23:01 PDT, Devin Rousso	no flags	Details Formatted Diff Diff
[Image] After Patch is applied (53.68 KB, image/png) 2017-09-15 23:01 PDT, Devin Rousso	no flags	Details
Patch (23.47 KB, patch) 2017-09-15 23:41 PDT, Devin Rousso	no flags	Details Formatted Diff Diff
[Image] After Patch is applied (167.37 KB, image/png) 2017-09-15 23:41 PDT, Devin Rousso	no flags	Details
Patch (23.57 KB, patch) 2017-10-25 23:38 PDT, Devin Rousso	no flags	Details Formatted Diff Diff
Archive of layout-test-results from ews103 for mac-elcapitan (1.06 MB, application/zip) 2017-10-26 00:33 PDT, Build Bot	no flags	Details
Archive of layout-test-results from ews113 for mac-elcapitan (1.90 MB, application/zip) 2017-10-26 00:34 PDT, Build Bot	no flags	Details
Archive of layout-test-results from ews106 for mac-elcapitan-wk2 (1.23 MB, application/zip) 2017-10-26 07:18 PDT, Build Bot	no flags	Details
Patch (24.18 KB, patch) 2017-10-26 14:39 PDT, Devin Rousso	no flags	Details Formatted Diff Diff
Patch (24.17 KB, patch) 2017-11-02 15:55 PDT, Devin Rousso	no flags	Details Formatted Diff Diff
Show Obsolete (8) View All Add attachment proposed patch, testcase, etc.

Matt Baker

Comment 1 2017-09-15 22:56:22 PDT

It could be good to have a popover with larger thumbnail when clicking/hovering the swatch.

Devin Rousso

Comment 2 2017-09-15 23:01:21 PDT

Created attachment 320996 [details] Patch

Devin Rousso

Comment 3 2017-09-15 23:01:50 PDT

Created attachment 320997 [details] [Image] After Patch is applied

Devin Rousso

Comment 4 2017-09-15 23:41:16 PDT

Created attachment 320998 [details] Patch Add swatches for CanvasGradient/CanvasPattern in the DetailsSidebar

Devin Rousso

Comment 5 2017-09-15 23:41:43 PDT

Created attachment 320999 [details] [Image] After Patch is applied

Devin Rousso

Comment 6 2017-10-25 23:38:47 PDT

Created attachment 324966 [details] Patch Rebased.

Build Bot

Comment 7 2017-10-26 00:33:40 PDT

Comment on attachment 324966 [details] Patch Attachment 324966 [details] did not pass mac-ews (mac): Output: http://webkit-queues.webkit.org/results/4995206 New failing tests: inspector/canvas/recording-2d.html

Build Bot

Comment 8 2017-10-26 00:33:41 PDT

Created attachment 324971 [details] Archive of layout-test-results from ews103 for mac-elcapitan The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews103 Port: mac-elcapitan Platform: Mac OS X 10.11.6

Build Bot

Comment 9 2017-10-26 00:34:54 PDT

Comment on attachment 324966 [details] Patch Attachment 324966 [details] did not pass mac-debug-ews (mac): Output: http://webkit-queues.webkit.org/results/4995090 New failing tests: inspector/canvas/recording-2d.html

Build Bot

Comment 10 2017-10-26 00:34:55 PDT

Created attachment 324972 [details] Archive of layout-test-results from ews113 for mac-elcapitan The attached test failures were seen while running run-webkit-tests on the mac-debug-ews. Bot: ews113 Port: mac-elcapitan Platform: Mac OS X 10.11.6

Build Bot

Comment 11 2017-10-26 07:18:41 PDT

Comment on attachment 324966 [details] Patch Attachment 324966 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.webkit.org/results/4998205 New failing tests: inspector/canvas/recording-2d.html

Build Bot

Comment 12 2017-10-26 07:18:43 PDT

Created attachment 325008 [details] Archive of layout-test-results from ews106 for mac-elcapitan-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews106 Port: mac-elcapitan-wk2 Platform: Mac OS X 10.11.6

Devin Rousso

Comment 13 2017-10-26 14:39:19 PDT

Created attachment 325056 [details] Patch Added missing include for tests

Blaze Burg

Comment 14 2017-11-02 14:34:40 PDT

Comment on attachment 325056 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=325056&action=review r=me, nice work. > Source/WebInspectorUI/UserInterface/Base/ImageUtilities.js:59 > + WI.scratchCanvasContext2D = function(callback) { This is clever, but I don't like adding new functions to the top level namespace. I'd prefer a more boring design, like extracting this to a WI.ImageRenderer or WI.ImageUtilities class with static methods. Then if for some reason we need multiple scratchpads it is trivial to do so. let image = WI.ImageUtilities.imageFromImageData(data) let image = WI.ImageUtilities.imageFromScratchCanvas((context) => ...) > Source/WebInspectorUI/UserInterface/Views/RecordingStateDetailsSidebarPanel.js:144 > + let fragment = document.createDocumentFragment(); Nit: move closer to the use site. > Source/WebInspectorUI/UserInterface/Views/RecordingStateDetailsSidebarPanel.js:152 > + Nit: extra space > Source/WebInspectorUI/UserInterface/Views/RecordingStateDetailsSidebarPanel.js:156 > + Ditto.

Devin Rousso

Comment 15 2017-11-02 15:52:21 PDT

Comment on attachment 325056 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=325056&action=review >> Source/WebInspectorUI/UserInterface/Base/ImageUtilities.js:59 >> + WI.scratchCanvasContext2D = function(callback) { > > This is clever, but I don't like adding new functions to the top level namespace. I'd prefer a more boring design, like extracting this to a WI.ImageRenderer or WI.ImageUtilities class with static methods. Then if for some reason we need multiple scratchpads it is trivial to do so. > > let image = WI.ImageUtilities.imageFromImageData(data) > let image = WI.ImageUtilities.imageFromScratchCanvas((context) => ...) Will address in followup to avoid making this more complex. <https://webkit.org/b/179207>

Devin Rousso

Comment 16 2017-11-02 15:55:03 PDT

Created attachment 325787 [details] Patch

WebKit Commit Bot

Comment 17 2017-11-02 17:54:38 PDT

Comment on attachment 325787 [details] Patch Clearing flags on attachment: 325787 Committed r224367: <https://trac.webkit.org/changeset/224367>

WebKit Commit Bot

Comment 18 2017-11-02 17:54:40 PDT

All reviewed patches have been landed. Closing bug.

Radar WebKit Bug Importer

Comment 19 2017-11-15 13:07:05 PST

<rdar://problem/35568817>

Note You need to log in before you can comment on or make changes to this bug.