Merged Imagedraw rewrite #737
Conversation
The functions `polygon8`, `polygon32` and `polygon32rgba` all have exactly the same logic in code, only changes the hline function called inside. Now all the logic is contained in `polygon_generic` which gets a function pointer to the hline handler, so there is no duplicated code anymore.
The rounds used in the library are towards positive/negative infinity. Since we're in a cartesian plane, rounds around zero are much more convenient, so we can use positive and negative values with consistent results.
The (previously refactored) polygon_generic function didn't draw
consistent polygons (equilateral polygons were not equilateral nor
symmetrical).
The most notable changes are:
* The horizontal edges are searched for when finding the polygon
boundaries, drawn and discarded from the edge list used to detect
intersections.
* The intersections are now checked and calculated from the current value
of the scan line (ymin) instead of in the middle (ymin + 0.5).
* Because the change in the scan line behavior, we should duplicate
the intersections in the maximum Y value of an edge or there will be
draw errors with concave and complex polygons.
* The rounds of the X coordinates in the hline function calls are
switched to draw the inner pixels.
* Removed the ugly micro-optimization of qsort at the end.
This implementation of the scan line algorithm may not be technically
correct, it's not optimized and it have problems with some edge cases,
like a wide line from (x0, y) to (x1, y + 1), therefore it should be
reviewed in the future.
The previous version of the function didn't generate correct wide lines
of even width.
The most notable changes are:
* Make variable names far more descriptive about the process.
* Corrected the width calculation; we should deduct one pixel from the
width because the pixels at the center of the line doesn't count for
the triangles.
* Now we calculate *two* ratios, one for left/top displacement (dxmin)
and one for right/bottom (dxmax), this fix the behavior with lines
of even width.
It can probably be optimized.
The method `create_base_image_draw` will be used in all the new draw tests.
The diagonals of the right angled edges must be perfect and the bottom vertice should be drawn.
Notice that the expansion of the line width depends on the order of the
points:
* If the bigger axis value is provided as the *second* point the line
expand first to the *positive* side of the axis.
* If the bigger axis value is provided as the *first* point the line
expand first to the *negative* side of the axis.
* If the line's width is odd this doesn't matter, as the line will
expand the same amount to both sides.
This behavior should be consistent in both horizontal and vertical lines.
The behavior is consistent with horizontal lines, see previous commit for details.
Now writing in the wall "don't push before compile" 100 times.
Only the oblique 3 pixels wide lines are defined: * The oblique 2 pixels wide lines are somewhat hard to define. * To define the oblique lines wider than 3 pixels we neet to define first how the oblique lines should expand their width (realistic or exact).
These tests should guarantee that the proportion of the width is maintained with a margin of error < 1%.
This tests are designed to guarantee that the wide lines behave exactly like normal lines drawn with the Bresenham's algorithm. This tests are somewhat subjective since this is non-defined behavior, but I think that mimic the Bresenham's algorithm is reliable enough. Currently the horizontal version of this test **fail**.
|
Remaining failure is the "line horizontal 1px slope 2px wide" test, which was mentioned in the original as an inconsistency between horizontal and vertical lines. |
|
The pieslice and polygon test images look immediately better, no more rogue pixels outside the border. |
|
I've now skipped the one failing test, I'll re-enable in a new PR/bug to fix that one behavior. IMO, this is an overall win for us, despite the failing test. I recommend merge before release. |
|
The 3.4 build fails when building |
|
Gah. I missed that with the other test failures. I'll fix. That will kill windows as well. It doesn't do that OMM. |
|
Changes Unknown when pulling 98a4991 on wiredfool:terseus_imagedraw into * on python-pillow:master*. |
|
Oops, sorry for coming too late, I didn't though this would be merged in its current state. Long story short, the one big rule when drawing polygons is, every pixel belong to one, and only one, polygon. I'll work in a new implementation of the polygon draw algorithm this week, sine this is now merged into mainline I'll send a new PR with it along with the tests adapted to the correct behavior. If someone wants to know more about this subject I recommend beginning with this article of gameprogrammer.com, although it's dated it's also one of the few articles I found which explain the most important aspects of polygon rendering. |
Continuation of #610, merged to master.