<div dir="ltr"><div>Hi,<br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Fri, Nov 1, 2019 at 9:48 PM Caolán McNamara <<a href="mailto:caolanm@redhat.com">caolanm@redhat.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">On Fri, 2019-11-01 at 13:40 +0100, Luboš Luňák wrote:<br>
> , the second and third rectangles miss their right and bottom edges.<br>
> The first one is correct, and it is because that one uses drawRect()<br>
<br>
There are so many rectangles there that I'm not sure I see the<br>
difference, but I know that the various ::drawRect implementations are<br>
all hacked to pull in the bottom right by a pixel for the line draw so<br>
drawRect is the "odd" case and drawPolyPolygon is the "normal" one.<br>
<br>
vcl/headless/svpgdi.cxx's drawRect just calls drawPolyPolygon with a<br>
-1,-1 for the line case. While X11SalGraphicsImpl::drawRect calls<br>
XDrawRectangle with a -1,-1, AquaSalGraphics::drawRect likewise and so<br>
on. So if they were left to their own "natural" behaviour the drawRect<br>
would presumably give the same results as drawPolyPolygon.<br></blockquote><div><br></div><div><div>The case in visualbackendtest corresponds to: <br></div><div>- BackendTest::testDrawFilledRectWithRectangle<br></div><div>- BackendTest::testDrawFilledRectWithPolygon</div><div><div>- BackendTest::testDrawFilledRectWithPolyPolygon</div>- BackendTest::testDrawFilledRectWithPolyPolygon2D</div><div><br></div><div>Looking into test case for testDrawFilledRectWithPolygon it draws a polygon for 2,2 -> 10,2 -> 10,10 -> 2,10 and from this setup I would say it should draw a filled polygon with pixel 10 inclusive, but that's not the case. I guess drawRect just compensates for this fact. In any way - either drawRect or drawPolygon (PolyPolygon, Polyline) is needs fixing and we need to make our expectations clear and document them (with the test and clarifying in the test).</div><div><br></div><div>I think the reason polygon draws it like this is because interpretation of floating-point coordinates is usually exclusive because that's what makes sense when you have sub-pixel positioning, but for integer coordinates (like in this case) it is inclusive and we are mixing one with the other on various levels. OTOH why doesn't this happen<br></div><div><br></div><div>Generally, I don't think this is a too much of a problem, because we usually draw with line + fill color, even when they are both set to the same color, but we need to set our expectations straight. The bad think because of this issue is that backends usually either "fill" or "stroke", so because of this problem here we need to actually do 2 drawing operations when we would only need 1. Generally not a problem, but with a more complicated polygons we are giving up performance.<br></div><div><br></div></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
Presumably the goal is to get the same results in the skia backend as<br>
the existing ones so if you get the same results I'd call it a job well<br>
done and leave it at that ;-) I imagine any deviation from how it works<br>
currently will just triggers regressions.<br>
<br></blockquote><div><br></div>Well, the BackendTests were created just for off-by-one errors and to make sure there are no differences in rendering between backends, but the tests also go through OutputDevice interface so the expectations of what should be drawn is clear.  I don't agree we should just do-it-like-everyone-else in this case (I think this is a very bad practice), we need to do it properly and test driven. <br></div><div class="gmail_quote"><br></div><div class="gmail_quote">The BackendTests are disabled currently for every backend because most of the backends show various rendering issues in various tests. I didn't have time to evaluate the results yet and investigate what the problems are and how to fix them or the test sets up wrong expectations.<br></div><div class="gmail_quote"><br></div><div class="gmail_quote">Regards, Tomaž<br></div></div>