[Webkit-unassigned] [Bug 83206] webkit fails IETC border-radius-content-edge-001
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Thu Aug 2 02:45:05 PDT 2012
https://bugs.webkit.org/show_bug.cgi?id=83206
--- Comment #15 from Takashi Sakamoto <tasak at google.com> 2012-08-02 02:45:04 PST ---
(From update of attachment 155778)
View in context: https://bugs.webkit.org/attachment.cgi?id=155778&action=review
Thank you for reviewing. I added a new method by extending an existing getRoundedBorderFor.
>> Source/WebCore/ChangeLog:10
>> + result with a result obtained by using Firefox.
>
> And what was the result of the comparison?
I see. I added "the result was the same".
>> Source/WebCore/ChangeLog:20
>> + doesn't include any padding), the radii's scale of the calculrated
>
> typo: calculated.
Done.
>> Source/WebCore/ChangeLog:24
>> + causes to shrink the radii too much.
>
> This probably should go in the LayoutTest ChangeLog as it describes why the image is failing. Please file a bug about that before landing and mention that it is covered by this bug in the ChangeLog.
This is caused by fighting APIs out of RenderStyle.cpp. I removed inline fighting codes and added a new API for getting rounded rect for content. So I removed this comment.
>> Source/WebCore/rendering/RenderImage.cpp:450
>> + GraphicsContextStateSaver clipToBorderStateSaver(*context, hasRoundedBorder);
>
> This should be moved inside the if below and you can remove the |hasRoundedBorder| check.
I think, GraphicsContextStateSaver rollbacks the rounded clipping in its destructor. So I have to place the GraphicsContext in the same level as drawImage.
>> Source/WebCore/rendering/RenderImage.cpp:454
>> + rectWithBorder.expand(borderLeft() + borderRight(), borderTop() + borderBottom());
>
> I feel like you are fighting the API here. getRoundedInnerBorderFor will re-add the borders back in the case where no padding was set on your image. My bet is that you can just pass the args as |false| but nothing prevents you from actually making a variant that works in your case, instead of implementing everything inline here.
I added a new method, getRoundedContentFor.
>> Source/WebCore/rendering/RenderImage.cpp:461
>> + radii.shrink(paddingBorderBox.before(style()), paddingBorderBox.after(style()), paddingBorderBox.start(style()), paddingBorderBox.end(style()));
>
> This line is wrong. You need to use the physical coordinates (top / left / ...) not the logical one (before / after / ...). You would see that if you had a test case involving some vertical writing-mode, which I already advised you to add but you missed the cue.
>
> The easiest way is to add a new class to your test similar to #border_radius_padding but which has: -webkit-writing-mode: vertical-rl;
I see. I added new tests for checking vertical writing-mode.
>> LayoutTests/fast/borders/border-radius-image.html:27
>> +<body>
>
> A test should explain what you are trying to achieve. Here it's extremely difficult to do so.
>
> Some questions that should be answered:
> * What is expected here?
> * What are you even testing?
>
> As you are using a pixel test, you don't want that to be dumped as text in the image. There are several tricks on http://trac.webkit.org/wiki/Writing%20Layout%20Tests%20for%20DumpRenderTree#Pixeltesttips on how to do that.
I see. I added descriptions about these tests. As I would like to use only pixel test, I added the descriptions as comments and added dumpAsText(true).
--
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
More information about the webkit-unassigned
mailing list