This is the next step after https://bugs.webkit.org/show_bug.cgi?id=44006. We should save and restore shadow parameters properly. We will not use GraphicsContextState for this because we may want to cache some stuff useful for shadow blur.
Created attachment 64459 [details] Patch
Attachment 64459 [details] did not pass style-queue: Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1 WebCore/platform/graphics/qt/GraphicsContextQt.cpp:177: This { should be at the end of the previous line [whitespace/braces] [4] Total errors found: 1 in 2 files If any of these errors are false positives, please file a bug against check-webkit-style.
Attachment 64459 [details] did not build on qt: Build output: http://queues.webkit.org/results/3778194
Created attachment 64461 [details] Patch
(In reply to comment #3) > Attachment 64459 [details] did not build on qt: > Build output: http://queues.webkit.org/results/3778194 This is bogus, since this patch requires https://bugs.webkit.org/show_bug.cgi?id=44015 first to be applied.
Attachment 64461 [details] did not build on qt: Build output: http://queues.webkit.org/results/3717221
(In reply to comment #6) > Attachment 64461 [details] did not build on qt: > Build output: http://queues.webkit.org/results/3717221 Please ignore this, the patch can't be built before https://bugs.webkit.org/show_bug.cgi?id=44015,
Comment on attachment 64461 [details] Patch > +2010-08-15 Ariya Hidayat <ariya@sencha.com> > + > + Reviewed by NOBODY (OOPS!). > + > + [Qt] Save and restore shadow state in GraphicsContextQt > + https://bugs.webkit.org/show_bug.cgi?id=44031 > + > + * platform/graphics/qt/GraphicsContextQt.cpp: Ariya, could you enrich you ChangeLog? > + > +// This is to track and keep the shadow state. We use this rather than > +// using GraphicsContextState to allow possible optimizations (right now > +// only to determine the shadow type, but in future it might covers things > +// like cached scratch image, persistent shader, etc). /s/covers/cover > + : color(c) > + , blurRadius(qRound(r)) > + , offset(dx, dy) > + { > + // The type of shadow is decided by the blur radius, shadow offset, and shadow color. > + if (!color.isValid() || !color.alpha()) { > + // Can't paint the shadow with invalid or invisible color. > + type = NoShadow; > + } else { > + if (r > 0) { > + // Shadow is always blurred, even the offset is zero. > + type = BlurShadow; > + } else { > + if (offset.isNull()) { > + // Without blur and zero offset means the shadow is fully hidden. > + type = NoShadow; > + } else { > + if (color.alpha() > 0) > + type = AlphaSolidShadow; > + else > + type = OpaqueSolidShadow; > + } > + } > + } > + } If you use "else if" instead of "else { \n\TAB\ if .." you would look better, identation-wise. > + > + void clear() > + { > + type = NoShadow; > + color = QColor(); > + blurRadius = 0; > + offset = QPointF(0, 0); Lets use QPoingF() here. r=me with the above fixed! Nice split up / group up.
Comment on attachment 64461 [details] Patch Manually committed r65393: http://trac.webkit.org/changeset/65393
Reverted r65393 for reason: Breaks some canvas tests Committed r65394: <http://trac.webkit.org/changeset/65394>
http://trac.webkit.org/changeset/65393 might have broken Qt Linux Release
Created attachment 64468 [details] Patch
Comment on attachment 64468 [details] Patch The updated patch should fix the problem with canvas tests (the reason I rolled out the previous patch, http://trac.webkit.org/changeset/65394). Here is the small diff of the fix: diff --git a/WebCore/platform/graphics/qt/GraphicsContextQt.cpp b/WebCore/platform/graphics/qt/GraphicsContextQt.cpp index 43fc32f..d30ea66 100644 --- a/WebCore/platform/graphics/qt/GraphicsContextQt.cpp +++ b/WebCore/platform/graphics/qt/GraphicsContextQt.cpp @@ -920,9 +920,10 @@ void GraphicsContext::setPlatformShadow(const FloatSize& size, float blur, const // Meaning that this graphics context is associated with a CanvasRenderingContext // We flip the height since CG and HTML5 Canvas have opposite Y axis m_common->state.shadowSize = FloatSize(size.width(), -size.height()); + m_data->shadow = ContextShadowParameter(color, blur, size.width(), -size.height()); + } else { + m_data->shadow = ContextShadowParameter(color, blur, size.width(), size.height()); } - - m_data->shadow = ContextShadowParameter(color, blur, size.width(), size.height()); }
Comment on attachment 64468 [details] Patch r=me
Comment on attachment 64468 [details] Patch Clearing flags on attachment: 64468 Committed r65420: <http://trac.webkit.org/changeset/65420>
All reviewed patches have been landed. Closing bug.