search

holoviz / datashader

Quickly and accurately render even the largest data.
http://datashader.org
BSD 3-Clause "New" or "Revised" License
3.3k stars 366 forks source link

Remove artifact from Polygon rendering #1329

Closed hoxbro closed 4 months ago

hoxbro commented 4 months ago

Fixes https://github.com/holoviz/datashader/issues/1327

image

image

Lets see how the CI handles this change.

Edit: Can still see some artifacts, if I use the holoviews example in the original issue. Can make them disappear, if I completely remove this if statement.

jbednar commented 4 months ago

Thanks for taking a stab at this! It does seem to address the zoomed-out issue:

image

But zooming in, seems like it's not actually drawing the polygons any more?

image
jbednar commented 4 months ago

For reference, here's what it did before the change, where the zoomed-out version has artifacts, but the zoomed-in version is fine:

image
hoxbro commented 4 months ago

image

jbednar commented 4 months ago

Looks great! Panning and zooming with HoloViews, I can no longer detect any issues now for that dataset. Thanks for following up on that @Hoxbro ! Can you motivate what the meaning of each of the changes is, and any risks?

To me it seems like one risk would be if the polygon coordinates are near the atol value for np.isclose, which is 1e-08 by default. E.g. if we were plotting nanometer-scale shapes on an integrated circuit wafer, would we have problems from the shape coordinates being considered "close"? Or are all such comparisons being done in pixel space, where 1e-08 is indeed a very small tolerance?

hoxbro commented 4 months ago

I have removed np.isclose, which I played around with when trying to find the problem. The coordinates is in pixel space, but I saw no difference with it. In theory, it could catch some of the cross-products, which are close to zero, but I don't feel it is worth the extra calculations for a case that right now is only theoretical.

For risk related to the removal of +1 I can't say as this is pretty deep in the machinery combined with some theory I haven't dove deep into.