-
-
Notifications
You must be signed in to change notification settings - Fork 371
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support compound reductions for antialiased lines on the CPU #1146
Support compound reductions for antialiased lines on the CPU #1146
Conversation
else: | ||
for k in range(n_aggs): | ||
_combine_in_place(accum_aggs[k], aggs[k], antialias_combinations[k]) | ||
_aa_stage_2_accumulate(aggs_and_accums, antialias_combinations) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note the increase in readability of this function and others like it by using separate jit-able functions like _aa_stage_2_accumulate
. This also allows functions like this to have their @expand_aggs_and_cols
decorators reinstated, in line with the other various low-level line rendering functions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for taking this on. My brain wants to crawl into a corner and die when I read this code (before and after this PR, so not your fault!).
Note the halos around crossing edges of min, first and last.
I can see those, but can't quite get ahold of the mathematics involved to see if there is any way around that.
mean loses the antialiased edges as it essentially performs a block fill of the pixels that would be rendered partially transparent. This is understandable from the maths of mean = sum / count. For a single antialiased line of a constant value the pixel values for sum equal count * value, hence mean = value.
Again, I'm not sure if there is a way around this? Seems like value should be e.g. half the actual value, for the smoothed bits of the edge?
Codecov Report
@@ Coverage Diff @@
## master #1146 +/- ##
==========================================
+ Coverage 85.20% 85.37% +0.17%
==========================================
Files 34 34
Lines 7732 7782 +50
==========================================
+ Hits 6588 6644 +56
+ Misses 1144 1138 -6
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
There is no workaround for this, it would need a more complicated implementation as we don't store the information required to do this. We would need to separate out the antialias For a compound reduction of a In general, we couldn't just have a single We would need to decide on appropriate mathematics to combine say In summary, to obtain a
|
Co-authored-by: James A. Bednar <jbednar@users.noreply.github.com>
You say there is no workaround, and then proceed to lay out a workaround. :-) Please open a separate issue proposing that work, emphasizing what limitations antialiasing will have until it is done, but then I don't think there's a particular reason to undertake it at this time. Do mention the limitation in the docs, though, and I guess link to the issue. Thanks! |
On rereading my previous comment it sounds more positive than I intended it to be! It is not a workaround because it doesn't work; I don't believe there is any maths that can combine |
This implements compound reductions such as
mean
andsummary
for antialiased lines on the CPU only. The only reductions not yet covered arestd
andvar
, but there is a possible solution for these being investigated. There is no support for antialiased lines on the GPU, and this will not occur soon as it requires significant changes.Demonstration of antialiased lines and various reductions:
Output images for

any
,count
andsum
min
andmax
first
,last
,mean
Two observations:
min
,first
andlast
.mean
loses the antialiased edges as it essentially performs a block fill of the pixels that would be rendered partially transparent. This is understandable from the maths ofmean = sum / count
. For a single antialiased line of a constantvalue
the pixel values forsum
equalcount * value
, hencemean = value
. Note that this is also subject to numerical rounding (not seen here) as typically an antialiasedcount
is calculated usingfloat32
whereas asum
usesfloat64
.Closes #1133.