[webkit-reviews] review denied: [Bug 136098] CanvasRenderingContext2D direction should reflect change in Element's dir attribute : [Attachment 236862] Patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Wed Aug 20 08:58:46 PDT 2014
Darin Adler <darin at apple.com> has denied Vivek Galatage <vivekg at webkit.org>'s
request for review:
Bug 136098: CanvasRenderingContext2D direction should reflect change in
Element's dir attribute
https://bugs.webkit.org/show_bug.cgi?id=136098
Attachment 236862: Patch
https://bugs.webkit.org/attachment.cgi?id=236862&action=review
------- Additional Comments from Darin Adler <darin at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=236862&action=review
Doesn’t work right with save/restore. After this sequence:
context.direction = "inherit";
context.save();
context.direction = "rtl";
context.restore();
the context will be in non-inherited state, but should be in inherited state.
The correct way to fix this is to change m_direction in State to have a
separate state for inherit, so perhaps not be a TextDirection at all, or to add
a boolean there, or have it be an Optional<TextDirection> or something like
that.
Need test cases that cover the behavior with save/restore.
> Source/WebCore/html/canvas/CanvasRenderingContext2D.cpp:124
> m_stateStack.first().m_direction = inheritedDirection(*canvas);
This is no longer a useful thing to do. It wastefully computes a direction that
will never be used since drawTextInternal will re-compute the direction every
time.
> Source/WebCore/html/canvas/CanvasRenderingContext2D.cpp:2294
> + if (m_hasInheritedDirection && computedStyle)
If computedStyle is null then we should use LTR, not the direction that already
happens to be in the state.
> Source/WebCore/html/canvas/CanvasRenderingContext2D.cpp:2295
> + modifiableState().m_direction = computedStyle->direction();
It’s not helpful to store a new direction in the context, since we just have to
recompute it every time anyway.
More information about the webkit-reviews
mailing list