Skip to content
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

fix linegaps #2828

Merged
merged 2 commits into from Mar 31, 2023
Merged

fix linegaps #2828

merged 2 commits into from Mar 31, 2023

Conversation

ffreyer
Copy link
Collaborator

@ffreyer ffreyer commented Mar 30, 2023

Description

Fixes #2826 for me on windows with arc.

Though given that this is the second issue with ifelse we come across we should probably just remove it all together :(

Type of change

  • Bug fix (non-breaking change which fixes an issue)

Checklist

  • Added an entry in NEWS.md (for new features and breaking changes)
  • Added or changed relevant sections in the documentation
  • Added unit tests for new algorithms, conversion methods, etc.
  • Added reference image tests for new plotting functions, recipes, visual options, etc.

@SimonDanisch
Copy link
Member

Oh yeah, I guess we gathered strong evidence for ifelse not working across gpus correctly :(

@ffreyer
Copy link
Collaborator Author

ffreyer commented Mar 30, 2023

    f_uv_minmax.x = step(segment_length, abs(length_a * dot(miter_a, v1))) * 
        ((u1 - g_thickness[1] * abs(dot(miter_a, v1) / dot(miter_a, n1))) * px2uv - f_uv_minmax.x) +
        f_uv_minmax.x;
    
    f_uv_minmax.y = step(segment_length, abs(length_b * dot(miter_b, v1))) *
        ((u2 + g_thickness[2] * abs(dot(miter_b, v1) / dot(miter_b, n1))) * px2uv - f_uv_minmax.y) +
        f_uv_minmax.y;

This also creates gaps on ym arc gpu, but maybe it works on other gpus? step has been a function forever so it should be pretty stable, right?

@MakieBot
Copy link
Collaborator

MakieBot commented Mar 30, 2023

Compile Times benchmark

Note, that these numbers may fluctuate on the CI servers, so take them with a grain of salt. All benchmark results are based on the mean time and negative percent mean faster than the base branch. Note, that GLMakie + WGLMakie run on an emulated GPU, so the runtime benchmark is much slower. Results are from running:

using_time = @ctime using Backend
# Compile time
create_time = @ctime fig = scatter(1:4; color=1:4, colormap=:turbo, markersize=20, visible=true)
display_time = @ctime Makie.colorbuffer(display(fig))
# Runtime
create_time = @benchmark fig = scatter(1:4; color=1:4, colormap=:turbo, markersize=20, visible=true)
display_time = @benchmark Makie.colorbuffer(display(fig))
using create display create display
GLMakie 37.45s (37.03, 37.65) 0.21+- 18.07s (17.67, 18.40) 0.25+- 16.58s (16.37, 16.92) 0.19+- 11.33ms (11.20, 11.45) 0.10+- 135.59ms (132.81, 136.94) 1.39+-
master 37.43s (37.20, 37.69) 0.18+- 18.05s (17.87, 18.26) 0.15+- 16.63s (16.20, 16.97) 0.31+- 11.37ms (11.11, 11.71) 0.19+- 136.57ms (134.39, 138.33) 1.37+-
evaluation +0.04%, 0.02s invariant (0.08d, 0.88p, 0.19std) +0.13%, 0.02s invariant (0.11d, 0.84p, 0.20std) -0.33%, -0.05s invariant (-0.21d, 0.70p, 0.25std) -0.34%, -0.04ms invariant (-0.25d, 0.65p, 0.15std) -0.72%, -0.98ms invariant (-0.71d, 0.21p, 1.38std)
CairoMakie 43.41s (41.52, 44.75) 1.35+- 23.94s (22.52, 25.15) 0.92+- 3.50s (3.29, 3.73) 0.15+- 17.43ms (17.12, 17.69) 0.17+- 7.84ms (7.26, 8.92) 0.55+-
master 42.87s (40.40, 45.16) 1.80+- 23.27s (21.75, 24.57) 0.98+- 3.50s (3.24, 3.68) 0.15+- 17.32ms (16.80, 17.57) 0.26+- 8.97ms (8.51, 9.23) 0.23+-
evaluation +1.23%, 0.53s invariant (0.33d, 0.54p, 1.58std) +2.81%, 0.67s invariant (0.71d, 0.21p, 0.95std) -0.25%, -0.01s invariant (-0.06d, 0.92p, 0.15std) +0.64%, 0.11ms invariant (0.50d, 0.37p, 0.22std) -14.48%, -1.13ms faster✅ (-2.68d, 0.00p, 0.39std)
WGLMakie 44.05s (43.73, 44.37) 0.19+- 21.89s (21.71, 22.03) 0.15+- 22.00s (21.58, 22.51) 0.35+- 14.28ms (14.02, 14.68) 0.24+- 787.66ms (770.43, 813.06) 15.52+-
master 43.89s (43.47, 44.21) 0.28+- 21.89s (21.58, 22.24) 0.25+- 22.04s (21.78, 22.46) 0.28+- 14.20ms (13.88, 14.63) 0.26+- 790.22ms (741.12, 845.58) 37.13+-
evaluation +0.36%, 0.16s invariant (0.66d, 0.24p, 0.24std) -0.00%, -0.0s invariant (-0.00d, 0.99p, 0.20std) -0.19%, -0.04s invariant (-0.13d, 0.81p, 0.32std) +0.56%, 0.08ms invariant (0.32d, 0.56p, 0.25std) -0.32%, -2.56ms invariant (-0.09d, 0.87p, 26.33std)

@ffreyer ffreyer marked this pull request as ready for review March 30, 2023 20:01
@SimonDanisch SimonDanisch merged commit ae12a0b into master Mar 31, 2023
12 of 13 checks passed
@SimonDanisch SimonDanisch deleted the ff/fix_lines branch March 31, 2023 09:22
@SimonDanisch
Copy link
Member

Thanks <3

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Broken trace in lines plot when figure stretched
3 participants