Bug 44025 - [Qt] Shadow blur for rectangle fill
Summary: [Qt] Shadow blur for rectangle fill
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P3 Normal
Assignee: Ariya Hidayat
URL:
Keywords: HTML5, Qt, QtTriaged
Depends on: 44015 44031
Blocks: 34479
  Show dependency treegraph
 
Reported: 2010-08-15 00:33 PDT by Ariya Hidayat
Modified: 2010-08-16 08:11 PDT (History)
1 user (show)

See Also:


Attachments
Patch (67.86 KB, patch)
2010-08-15 19:50 PDT, Ariya Hidayat
no flags Details | Formatted Diff | Diff
Patch (66.08 KB, patch)
2010-08-16 00:18 PDT, Ariya Hidayat
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Ariya Hidayat 2010-08-15 00:33:29 PDT
Implement blur support for shadow of rectangles.

Shadow pixel tests need to be updated, too.
Comment 1 Ariya Hidayat 2010-08-15 19:50:30 PDT
Created attachment 64463 [details]
Patch
Comment 2 Ariya Hidayat 2010-08-16 00:18:56 PDT
Created attachment 64469 [details]
Patch
Comment 3 Kenneth Rohde Christiansen 2010-08-16 04:25:33 PDT
Comment on attachment 64469 [details]
Patch

Looks good to me, some minor nits below.

WebCore/platform/graphics/qt/ContextShadow.cpp:70
 +  #define BLUR_SUM_SHIFT 14
Why not use a static const?

WebCore/platform/graphics/qt/ContextShadow.cpp:72
 +  // Note: image must be RGB32 format
You could add an assert for that, I guess

WebCore/platform/graphics/qt/ContextShadow.cpp:88
 +  
why this extra newline?

WebCore/platform/graphics/qt/ContextShadow.cpp:91
 +          int left, right, pixelCount, prev, next;
We normally do not add most variable declarations on the same line.

WebCore/platform/graphics/qt/ContextShadow.cpp:128
 +                          ((prev > 0) ? *prevPtr : firstAlpha);
why not just keep on one line :-) I guess it is shorter than the above for-one.

WebCore/platform/graphics/qt/ContextShadow.cpp:154
 +          // Step 3: blur green channel and store the result in the alpha channel
Add a dot at the end of the comment.

WebCore/platform/graphics/qt/ContextShadow.cpp:190
 +      // "colorize" with the right shadow color
here as well

WebCore/platform/graphics/qt/ContextShadow.cpp:220
 +      }
Here we shouldnt use braces according to the coding style guide
Comment 4 Ariya Hidayat 2010-08-16 08:02:21 PDT
Manually committed r65425: http://trac.webkit.org/changeset/65425
Comment 5 Ariya Hidayat 2010-08-16 08:11:08 PDT
Comment on attachment 64469 [details]
Patch

Clearing the review flag.