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

implement contour labels #2496

Merged
merged 32 commits into from
Apr 1, 2023
Merged

implement contour labels #2496

merged 32 commits into from
Apr 1, 2023

Conversation

t-bltg
Copy link
Collaborator

@t-bltg t-bltg commented Dec 13, 2022

Description

Fix #832.

Very early stage implementation, much to be discussed.

2D

using CairoMakie

main() = begin
  fig = let
    paraboloid = (x, y) -> 10(x^2 + y^2)
    x = range(-3, 3; length = 100)
    y = range(-3, 3; length = 200)
    z = paraboloid.(x, y')

    fig, ax, hm = heatmap(x, y, z; interpolate = true)
    Colorbar(fig[1, 2], hm)

    contour!(
      ax, x, y, z;
      color = :red, levels = 0:10:100, labels = true,
      labelsize = 15, labelfont = :bold, labelcolor = :orange
    )
    fig
  end

  save("contour_labels_2d.png", fig)
  fig
end

main() |> display

contour_labels_2d

3D

currently failing, transformation or projection issue, comments welcome :)

using CairoMakie

main() = begin
  fig = let
    fig = Figure()
    ax = Axis3(fig[1, 1])

    xs = ys = range(-.5, .5; length = 100)
    zs = @. (xs^2 + ys'^2)

    levels = .025:.05:.475
    contour3d!(-zs; levels = -levels, labels = true, color = :blue)  # labels inherit linecolor by default
    contour3d!(+zs; levels = +levels, labels = true, color = :red, labelcolor = :black)
    fig
  end

  save("contour_labels_3d.png", fig)
  fig
end

main() |> display

contour_labels_3d

  • rotate labels automagically to align on the local tangent ?
  • dynamic contour label attributes (fontsize, color, ...) ?
  • check fix contour3d

Type of change

Delete options that do not apply:

  • New feature (non-breaking change which adds functionality)

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.

@t-bltg t-bltg marked this pull request as draft December 13, 2022 13:13
@t-bltg t-bltg force-pushed the cont_lab branch 7 times, most recently from 126ea7c to 788b028 Compare December 13, 2022 15:41
@t-bltg t-bltg marked this pull request as ready for review December 13, 2022 15:53
@t-bltg t-bltg force-pushed the cont_lab branch 5 times, most recently from 0af7c32 to 6188208 Compare December 13, 2022 18:12
@t-bltg t-bltg marked this pull request as draft December 14, 2022 12:47
@t-bltg t-bltg force-pushed the cont_lab branch 3 times, most recently from 2571c43 to 785444c Compare December 14, 2022 18:10
@t-bltg
Copy link
Collaborator Author

t-bltg commented Dec 14, 2022

@jkrumbiegel, I'm a bit stuck in 3D.
The text bounding boxes are not correct, and I cannot find a way to fix it.
Using wireframe!(plot, bb, space = :pixel) as commented in the PR code draws the correct bboxes in the 2D examples, but renders nonsense in 3D.
I tried to lift(scene.camera.projectionview, scene.px_area) to recompute the bounding boxes but it fails.
I think it might be related to #1653, but I have no ideas left ...
Could you advise ?

@t-bltg t-bltg force-pushed the cont_lab branch 2 times, most recently from 6bb60a9 to 2cf1bfd Compare December 14, 2022 18:53
@jkrumbiegel
Copy link
Member

What do the wrong boundingboxes look like? Could also always still be a bug with space=:pixel I guess, unless you think the boundingboxes themselves are wrong somehow

@t-bltg

This comment was marked as outdated.

@t-bltg t-bltg force-pushed the cont_lab branch 4 times, most recently from 9033f17 to 44b628f Compare December 14, 2022 20:03
@t-bltg

This comment was marked as outdated.

@t-bltg
Copy link
Collaborator Author

t-bltg commented Dec 14, 2022

If I remove the scene.camera.projectionview, scene.px_area from the listeners on bboxes computation, then no bounding box is drawn in 3D.

This explains at least why the label masking in 3D doesn't appear: I think a masked line is drawn on top of an un-masked line (before the projection matrix changes, wrong bboxes), thus hiding the masking effect.

@jkrumbiegel
Copy link
Member

Ah Axis3 probably just doesn't ignore limits of plots with space=:pixel yet, there's still stuff to clean up.

On the second image the boxes don't look too bad right? Although not all of them are filled?

@lazarusA
Copy link
Contributor

oh, this is a good one!. A pitty that is still not ready to merge.

@t-bltg
Copy link
Collaborator Author

t-bltg commented Jan 26, 2023

What makes you say it isn't ?

@lazarusA
Copy link
Contributor

lazarusA commented Jan 26, 2023

apologies, maybe my mistake, I just saw the CI failing. And your 3d wireframe example, I don't see the current example working.

@asinghvi17
Copy link
Member

@t-bltg mind if I merge master in?

@t-bltg
Copy link
Collaborator Author

t-bltg commented Jan 26, 2023

apologies, maybe my mistake, I just saw the CI failing. And your 3d wireframe example, I don't see the current example working.

No worries.

The up-to-date examples (2D/3D) are in the first post (#2496 (comment)).

The failing CI is due to missing references images, which need to be merged by a maintainer.

@t-bltg t-bltg requested a review from jkrumbiegel April 1, 2023 14:44
@t-bltg
Copy link
Collaborator Author

t-bltg commented Apr 1, 2023

Pending CI, can we get this PR in ?

@SimonDanisch SimonDanisch reopened this Apr 1, 2023
@SimonDanisch SimonDanisch merged commit fb1f7f7 into MakieOrg:master Apr 1, 2023
@SimonDanisch
Copy link
Member

Thank you so much for this great PR :) Sorry for taking so long...

@t-bltg t-bltg deleted the cont_lab branch April 1, 2023 16:57
@t-bltg
Copy link
Collaborator Author

t-bltg commented Apr 1, 2023

Amazing, and many thanks to @jkrumbiegel for the hints and guidance towards (hopefully) quality code !

@t-bltg t-bltg mentioned this pull request Sep 8, 2023
7 tasks
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.

Feature request: contour labels
5 participants