Bug 162917

Summary: [Win][Direct2D] Add initial D2D GraphicsContext implementation
Product: WebKit Reporter: Brent Fulgham <bfulgham>
Component: WebCore Misc.Assignee: Brent Fulgham <bfulgham>
Status: RESOLVED FIXED    
Severity: Normal CC: achristensen, bfulgham, commit-queue, dino, ossy, pvollan
Priority: P2    
Version: WebKit Nightly Build   
Hardware: PC   
OS: All   
Bug Depends on:    
Bug Blocks: 161817    
Attachments:
Description Flags
Patch dino: review+

Description Brent Fulgham 2016-10-04 10:26:22 PDT
This patch adds a Direct2D GraphicsContext implementation, which will exist in parallel to the existing Cairo and CG implementations.

This patch simply adds the new files. It does not incorporate them into the build.
Comment 1 Brent Fulgham 2016-10-04 10:44:06 PDT
Created attachment 290615 [details]
Patch
Comment 2 WebKit Commit Bot 2016-10-04 10:45:02 PDT
Attachment 290615 [details] did not pass style-queue:


ERROR: Source/WebCore/platform/graphics/Color.h:48:  D2D1_VECTOR_4F is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/platform/graphics/win/ColorDirect2D.cpp:49:  D2D1_VECTOR_4F is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
Total errors found: 2 in 11 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 Dean Jackson 2016-10-04 10:49:57 PDT
Comment on attachment 290615 [details]
Patch

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

rs=me, although I feel like this is so much code it should go through real review with someone who knows D2D :|

> Source/WebCore/ChangeLog:12
> +        features on Windows using Direct2D.
> +
> +
> +        No new tests until complete backend lands.

Blank lines!

> Source/WebCore/platform/graphics/win/ColorDirect2D.cpp:56
> +}
> +
> +

More extra blank lines!

> Source/WebCore/platform/graphics/win/GraphicsContextDirect2D.cpp:177
> +        /*
> +        FIXME: Implement DisplayListRecorder support for drawNativeImage
> +        m_displayListRecorder->drawNativeImage(image, imageSize, destRect, srcRect, op, blendMode, orientation);
> +        */

Why not // comments?

> Source/WebCore/platform/graphics/win/GraphicsContextDirect2D.cpp:214
> +    // ImageOrientation expects the origin to be at (0, 0)

Nit: End with .

> Source/WebCore/platform/graphics/win/GraphicsContextDirect2D.cpp:839
> +        // ShadowBlur contextShadow(m_state);
> +        // contextShadow.drawRectShadow(*this, FloatRoundedRect(rect));

Does this need a FIXME?

> Source/WebCore/platform/graphics/win/GraphicsContextDirect2D.cpp:865
> +        // ShadowBlur contextShadow(m_state);
> +        // contextShadow.drawRectShadow(*this, FloatRoundedRect(rect));
> +        notImplemented();

Same here.

> Source/WebCore/platform/graphics/win/GraphicsContextDirect2D.cpp:890
> +        // ShadowBlur contextShadow(m_state);
> +        // contextShadow.drawRectShadow(*this, rect);
> +        notImplemented();

And here.

> Source/WebCore/platform/graphics/win/GraphicsContextDirect2D.cpp:960
> +        // ShadowBlur contextShadow(m_state);
> +        // contextShadow.drawRectShadow(*this, rect);
> +        notImplemented();

And probably many other places
Comment 4 Brent Fulgham 2016-10-04 10:54:00 PDT
Comment on attachment 290615 [details]
Patch

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

>> Source/WebCore/ChangeLog:12
>> +        No new tests until complete backend lands.
> 
> Blank lines!

Whoops!

>> Source/WebCore/platform/graphics/win/ColorDirect2D.cpp:56
>> +
> 
> More extra blank lines!

:-(.  Fixed.

>> Source/WebCore/platform/graphics/win/GraphicsContextDirect2D.cpp:177
>> +        */
> 
> Why not // comments?

Sure! Done.

>> Source/WebCore/platform/graphics/win/GraphicsContextDirect2D.cpp:214
>> +    // ImageOrientation expects the origin to be at (0, 0)
> 
> Nit: End with .

OK!

>> Source/WebCore/platform/graphics/win/GraphicsContextDirect2D.cpp:839
>> +        // contextShadow.drawRectShadow(*this, FloatRoundedRect(rect));
> 
> Does this need a FIXME?

Sure. And all the other places. Done.

>> Source/WebCore/platform/graphics/win/GraphicsContextDirect2D.cpp:865
>> +        notImplemented();
> 
> Same here.

Done.

>> Source/WebCore/platform/graphics/win/GraphicsContextDirect2D.cpp:890
>> +        notImplemented();
> 
> And here.

Done.

>> Source/WebCore/platform/graphics/win/GraphicsContextDirect2D.cpp:960
>> +        notImplemented();
> 
> And probably many other places

Done.
Comment 5 Brent Fulgham 2016-10-04 10:55:05 PDT
(In reply to comment #3)
> Comment on attachment 290615 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=290615&action=review
> 
> rs=me, although I feel like this is so much code it should go through real
> review with someone who knows D2D :|

Sadly, there seems to be no such person. I'm hoping one of the EA or Sony people will jump in at some point with some advice. Until then, I just want to get this prototype in place so others can build it and hack on it.
Comment 6 Brent Fulgham 2016-10-04 11:33:49 PDT
Committed r206773: <http://trac.webkit.org/changeset/206773>
Comment 7 Csaba Osztrogon√°c 2016-10-04 12:22:03 PDT
(In reply to comment #6)
> Committed r206773: <http://trac.webkit.org/changeset/206773>

It broke the Windows build, see build.webkit.org for details.
Comment 8 Brent Fulgham 2016-10-04 12:45:39 PDT
(In reply to comment #7)
> (In reply to comment #6)
> > Committed r206773: <http://trac.webkit.org/changeset/206773>
> 
> It broke the Windows build, see build.webkit.org for details.

Build correction landed:

Committed r206780: <http://trac.webkit.org/changeset/206780>