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

Beta 0.20 #2831

Closed
wants to merge 106 commits into from
Closed

Beta 0.20 #2831

wants to merge 106 commits into from

Conversation

jkrumbiegel
Copy link
Member

@jkrumbiegel jkrumbiegel commented Mar 31, 2023

This branch contains changes for the upcoming version v0.20, which we want people to try out before fully switching (therefore the "beta" status).

This branch contains the high-dpi work for GLMakie and the ability to show correctly scaled png images in web contexts such as Pluto, VSCode, Jupyter, Quarto etc. Because these changes eliminate the disparity between Mac and the other OSes, we can finally make adjustments to the general theme to account for that. So far, window size was a compromise so it wouldn't be too small on Mac and too large on Windows/Linux (at standard dpi values). Now we scale it down so it is more useable across a wide range of contexts, whether in documents or in interactive windows.

TODOs:

  • Antialiasing of lines scales with px_per_unit in GLMakie, making lines more blurry than necessary

asinghvi17 and others added 30 commits January 16, 2023 20:27
Also allows NaN points to propagate ahead from surface to mesh.

Solves https://www.github.com/MakieOrg/GeoMakie.jl/issues/133
Basically replicates what's done in GLMakie's utils shader.  Skips any combination of points which has a NaN when computing normals.
Simplify the code a lot as well.
removed an extra 'end'
Co-committed-by: Frederic Freyer <frederic481994@hotmail.de>
can be achieved by removing hotkeys

# decrease the scale factor after-the-fact
screen.scalefactor[] = 1
@test GLMakie.window_size(screen.glscreen) == scaled(screen, (W, H))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This fails for me on ubuntu

  Expression: GLMakie.window_size(screen.glscreen) == scaled(screen, (W, H))
   Evaluated: (800, 800) == (400, 400)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Running it manually works though. Just not in ]test GLMakie...?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yield() fixes it 🤷

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No it doesn't. It's failing again after merging another pr locally

@github-actions
Copy link
Contributor

Missing reference images

Found 6 new images without existing references.
Upload new reference images before merging this PR.

@ffreyer
Copy link
Collaborator

ffreyer commented Jul 12, 2023

Closes #2598

I tried fixing the line AA stuff but I'm not sure if I have the correct idea on how it should look. I basically tried to match lines with half resolution and linewidth at 200% zoom with px_per_unit = 0.5.

Examples

Screenshot from 2023-07-12 16-58-15
Screenshot from 2023-07-12 17-23-57
Screenshot from 2023-07-12 17-24-23
Screenshot from 2023-07-12 17-24-33
Screenshot from 2023-07-12 17-25-35
Screenshot from 2023-07-12 17-25-44

@github-actions
Copy link
Contributor

Missing reference images

Found 7 new images without existing references.
Upload new reference images before merging this PR.

@ffreyer
Copy link
Collaborator

ffreyer commented Jul 12, 2023

Closes #2746

@github-actions
Copy link
Contributor

Missing reference images

Found 7 new images without existing references.
Upload new reference images before merging this PR.

@@ -65,6 +65,7 @@ end
markersize=size,
axis = (; scenekw = (;limits=Rect3f(Point3(0), Point3(1))))
)
update_cam!(ax.scene, Point3f(2.224431, 2.224431, 2.128731), Point3f(0.5957, 0.5957, 0.50000006))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Highlighting this here too in case we want to undo it. This sets the camera orientation to match pre #2746. The change is pretty minimal but gets picked up easily in refimages.

@github-actions
Copy link
Contributor

Missing reference images

Found 7 new images without existing references.
Upload new reference images before merging this PR.

@jkrumbiegel
Copy link
Member Author

@ffreyer line antialiasing looks perfect now, thanks! Here with px_per_unit = 10:

grafik

@SimonDanisch
Copy link
Member

SimonDanisch commented Jul 18, 2023

TODOs to bring this to the finish line:

  • fix flaky test @test GLMakie.window_size(screen.glscreen) == scaled(screen, (W, H))
  • Merge axis type inferences from breaking-release combined #2220
  • Test on different resolution screens, IJulia, Pluto, VSCode, Windowed
  • rebase to only have merge commits from the PRs
  • investigate unexpected speed ups
  • reset camera settings from tests
  • check doc image generation

@ffreyer
Copy link
Collaborator

ffreyer commented Jul 31, 2023

Fixes #426 via #2598

@ffreyer
Copy link
Collaborator

ffreyer commented Jul 31, 2023

Also closes #2522?

@SimonDanisch SimonDanisch mentioned this pull request Aug 1, 2023
16 tasks
@SimonDanisch
Copy link
Member

Closing in favor of: #3113

SimonDanisch added a commit that referenced this pull request Nov 17, 2023
Continues #2831 !
Still needs to check, if I rebased correctly and didn't incorrectly
apply some of the changes!

## Merged PRs
- #2598
- #2746
- #2346
- #2544
- #3082
- #2868
- #3062
- #3106
- #3281
- #3246

## TODOS

- [x] fix flaky test `@test GLMakie.window_size(screen.glscreen) ==
scaled(screen, (W, H))`
- [x] Merge axis type inferences from #2220 
- [x] Test on different resolution screens, IJulia, Pluto, VSCode,
Windowed
- [x] rebase to only have merge commits from the PRs 
- [x] investigate unexpected speed ups
- [x] reset camera settings from tests
- [ ] check doc image generation
- [x] rethink default near/far in Camera3D (compatability with OIT)
- [x] merge #3246
- [x] fix WGLMakie issues/tests:
- [x] fix line depth issues (see tests: ~~hexbin colorrange~~ (not new),
LaTeXStrings in Axis3, Axis3 axis reversal)
  - [x] fix lighting of surface with nan points (fixed in #3246)
- ~~volume/3D contour artifacts (see 3D Contour with 2D contour
slices)~~ not new
  - ~~artifacting in "colorscale (lines)"~~ not new
- [x] GLMakie:
  - [x] slight outline in "scatter image markers" test
  - ~~clipping/z-fighting in "volume translated"~~ not new
- [x] CairoMakie:
  -  ~~Artfiacting in `colorscale (lines)"~~ not new
  - ~~markersize in "scatter rotations" changed?~~ not new
  - ~~color change in "colorscale (poly)"~~ not new
  - ~~transparency/render order of "OldAxis + Surface"~~ not new
  - ~~render order in "Merged color mesh"~~ not new
  - ~~render order of "Surface + wireframe + contour"~~ not new
- [x] Check "SpecApi in convert_arguments" (colors swapped?)


## Fixes the following errors

- fixes #2721 via #2746
- fixes #1600 via #2746
- fixes #1236 via #2746
- fixes MakieOrg/GeoMakie.jl#133 via #2598
- closes #2522
- closes #3239 via #3246
- fixes #3238 via #3246
- fixes #2985 via #3246
- fixes #3307 via #3281
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.

None yet

5 participants