Bug 212477

Summary:

Extended Color: ColorMatrix should support smaller matrices and be constexpr

Product:

WebKit

Reporter:

Sam Weinig <sam>

Component:

Platform

Assignee:

Sam Weinig <sam>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

dino, ews-watchlist, kondapallykalyan, simon.fraser, webkit-bug-importer

Priority:

Keywords:

InRadar

Version:

WebKit Nightly Build

Hardware:

Unspecified

OS:

Unspecified

Attachments:

Description	Flags
proof of concept	none
matrix.cpp	none
proof of concept	none
Patch	none
Patch	none
Patch	none
Patch	none
Patch	none

Sam Weinig

Reported 2020-05-28 10:35:41 PDT

The current ColorMatrix code always uses a 4x5 matrix. This is unnecessary for most (if not all) of the uses. We can do better by templatizing it and while we are at it, making it support constexpr.

Attachments
proof of concept (15.16 KB, text/plain) 2020-05-28 10:37 PDT, Sam Weinig	no flags	Details
matrix.cpp (4.16 KB, text/x-csrc) 2020-05-28 10:38 PDT, Sam Weinig	no flags	Details
proof of concept (4.25 KB, text/x-csrc) 2020-05-28 11:05 PDT, Sam Weinig	no flags	Details
Patch (31.70 KB, patch) 2020-05-28 14:11 PDT, Sam Weinig	no flags	Details Formatted Diff Diff
Patch (31.98 KB, patch) 2020-05-28 16:25 PDT, Sam Weinig	no flags	Details Formatted Diff Diff
Patch (31.66 KB, patch) 2020-05-28 17:55 PDT, Sam Weinig	no flags	Details Formatted Diff Diff
Patch (31.71 KB, patch) 2020-05-29 08:28 PDT, Sam Weinig	no flags	Details Formatted Diff Diff
Patch (33.29 KB, patch) 2020-05-29 10:10 PDT, Sam Weinig	no flags	Details Formatted Diff Diff
Show Obsolete (6) View All Add attachment proposed patch, testcase, etc.

Sam Weinig

Comment 1 2020-05-28 10:37:32 PDT

Created attachment 400476 [details] proof of concept Here is a proof-of-concept. Not sure if the actual matrix match is 100% right yet, but the code nicely compiles down to: :-) samweinig:~> otool -Vx a.out a.out: (__TEXT,__text) section _main: 0000000100000f4a pushq %rbp 0000000100000f4b movq %rsp, %rbp 0000000100000f4e leaq 0x53(%rip), %rdi ## literal pool for: "%f, %f, %f, %f\n" 0000000100000f55 movsd 0x3b(%rip), %xmm0 0000000100000f5d movsd 0x3b(%rip), %xmm3 0000000100000f65 movaps %xmm0, %xmm1 0000000100000f68 movaps %xmm0, %xmm2 0000000100000f6b movb $0x4, %al 0000000100000f6d callq 0x100000f76 ## symbol stub for: _printf 0000000100000f72 xorl %eax, %eax 0000000100000f74 popq %rbp 0000000100000f75 retq when compiled with `clang++ -std=c++17 -Os matrix.cpp`

Sam Weinig

Comment 2 2020-05-28 10:38:36 PDT

Created attachment 400477 [details] matrix.cpp

Sam Weinig

Comment 3 2020-05-28 11:05:09 PDT

Created attachment 400482 [details] proof of concept

Dean Jackson

Comment 4 2020-05-28 12:45:47 PDT

This is awesome.

Dean Jackson

Comment 5 2020-05-28 12:47:32 PDT

Our Color Team is on fire these days.