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

Add px_per_unit to GLMakie #2529

Closed
wants to merge 2 commits into from
Closed

Add px_per_unit to GLMakie #2529

wants to merge 2 commits into from

Conversation

ffreyer
Copy link
Collaborator

@ffreyer ffreyer commented Dec 27, 2022

Description

This is a prototype for #2522 though I don't have a mac to test anything with. Feel free to mess around with this @jkrumbiegel

The pr adds px_per_unit to the GLMakie screen config, which acts as a multiplier between the window size and the framebuffer size. So with this you can have 2x2 pixels per window resolution pixel by doing

...
display(GLMakie.Screen(px_per_unit = 2), fig)

TODO:

  • fix the black outline of LScenes with ssao
  • check if any of the screen/framebuffer size calls are expensive and if so avoid them (as far as I can tell GLFW.Get... calls are fairly expensive, but getting the size of a texture is now)
  • general cleanup

Type of change

  • 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.

@MakieBot
Copy link
Collaborator

MakieBot commented Dec 27, 2022

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 29.76s (29.56, 30.06) 0.18+- 16.47s (16.36, 16.61) 0.09+- 15.13s (15.10, 15.18) 0.03+- 10.74ms (10.61, 10.89) 0.10+- 115.36ms (114.43, 116.73) 0.74+-
master 29.75s (29.58, 29.91) 0.11+- 16.45s (16.41, 16.53) 0.04+- 15.14s (14.98, 15.31) 0.10+- 10.68ms (10.59, 10.79) 0.07+- 115.09ms (114.46, 115.70) 0.46+-
evaluation +0.04%, 0.01s invariant (0.08d, 0.89p, 0.15std) +0.12%, 0.02s invariant (0.27d, 0.62p, 0.07std) -0.07%, -0.01s invariant (-0.14d, 0.80p, 0.07std) +0.52%, 0.06ms invariant (0.65d, 0.25p, 0.08std) +0.23%, 0.26ms invariant (0.42d, 0.45p, 0.60std)
CairoMakie
master
evaluation
WGLMakie 40.03s (39.71, 40.63) 0.29+- 28.89s (28.55, 29.32) 0.32+- 44.33s (43.62, 45.14) 0.59+- 15.96ms (14.92, 17.30) 0.80+- 2.77s (2.71, 2.81) 0.04+-
master 40.48s (40.05, 41.21) 0.43+- 29.30s (28.75, 30.02) 0.51+- 44.39s (43.73, 45.84) 0.75+- 15.28ms (14.48, 16.18) 0.62+- 2.58s (2.55, 2.64) 0.03+-
evaluation -1.12%, -0.45s faster ✓ (-1.23d, 0.04p, 0.36std) -1.43%, -0.41s invariant (-0.98d, 0.10p, 0.41std) -0.13%, -0.06s invariant (-0.09d, 0.88p, 0.67std) +4.27%, 0.68ms invariant (0.95d, 0.10p, 0.71std) +6.61%, 0.18s slower❌ (5.36d, 0.00p, 0.03std)

@jkrumbiegel
Copy link
Member

Nice! Thank you :) it currently fails during loading though, same error as in CI

@jkrumbiegel
Copy link
Member

jkrumbiegel commented Dec 27, 2022

Ok so this doesn't quite solve the problem, yet, as the size of the elements is still too small, although I can now render it at higher quality. Here's an example with px_per_unit = 0.5, going the wrong way to make it visible, the apparent size of everything is still the same, it's just blurrier.

grafik

In the end, I need the same apparent scaling on a normal dpi monitor with 800x600 pixels, where text is 16px high, just with twice the pixels.

@ffreyer
Copy link
Collaborator Author

ffreyer commented Dec 27, 2022

Should the window just be double the size?l

@jkrumbiegel
Copy link
Member

Right now it is such that when a retina factor is detected, the window size is reduced by the same factor. For example, 800x600 figure gets a 400x300 size window, backed by a 800x600 pixel buffer. What I really need is a 800x600 window with a 1600x1200 buffer, but with the same "real estate" for the figure as in a non-retina window, just sharper.

end
return
end
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I thought the reduction in window size was happening here...

Looking at GLFW docs, there seems to be a retina window hint which we don't have in GLFW.jl

GLFW_COCOA_RETINA_FRAMEBUFFER specifies whether to use full resolution framebuffers on Retina displays. Possible values are GLFW_TRUE and GLFW_FALSE. This is ignored on other platforms.

https://www.glfw.org/docs/latest/window_guide.html

and there is also a compile flag

GLFW_USE_RETINA determines whether windows will use the full resolution of Retina displays.

https://www.glfw.org/docs/3.1/compile.html

Maybe we also need some changes in GLFW?

Copy link
Member

Choose a reason for hiding this comment

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

I think retina is already used correctly, you specify a window size of 400x300 and you get a frame buffer size of 800x600. It's just that the size of everything should be doubled in pixels so that it looks normal.

You can use the retina flag to disable that behavior, then you'd get a pixelated window though, I think.

Comment on lines -288 to -289
fb = screen.framebuffer
w, h = size(fb)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

There is also a window hint

GLFW_SCALE_TO_MONITOR specified whether the window content area should be resized based on the monitor content scale of any monitor it is placed on. This includes the initial placement when the window is created. Possible values are GLFW_TRUE and GLFW_FALSE.

which we do have available. You could try adding it here

windowhints = [
(GLFW.SAMPLES, 0),
(GLFW.DEPTH_BITS, 0),
# SETTING THE ALPHA BIT IS REALLY IMPORTANT ON OSX, SINCE IT WILL JUST KEEP SHOWING A BLACK SCREEN
# WITHOUT ANY ERROR -.-
(GLFW.ALPHA_BITS, 8),
(GLFW.RED_BITS, 8),
(GLFW.GREEN_BITS, 8),
(GLFW.BLUE_BITS, 8),
(GLFW.STENCIL_BITS, 0),
(GLFW.AUX_BUFFERS, 0),
]

If this changes the size of the final framebuffer you will probably need to adjust this to something like 2 .* size(screen). (I'm guessing GLFW.GetFramebufferSize(screen.glscreen) will return the correct size, but it would be better to avoid calling that all the time. If we don't need px_per_unit we can probably just revert to the old code.)

Copy link
Member

Choose a reason for hiding this comment

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

I don't have a second monitor to test with, but it sounds reasonable that pixel density should be automatic and depend on the current monitor. Unless you want to override it, or render out hi res pngs.

@jkrumbiegel
Copy link
Member

jkrumbiegel commented Dec 28, 2022

I found some code that looks like it solves the same problem: https://web.eecs.umich.edu/~sugih/courses/eecs487/glut-howto/glfw/sample.cpp

This does the "scaling" that I was talking about if I understand correctly, with the glOrtho(0.0, (float)fbwidth, 0.0, (float)fbheight, -1.f, 1.f); call.

/* Called when framebuffer is resized, e.g., when window is resized
 * OR when the same size buffer is moved across Retina and non-Retina displays
 * when running Mac OS X.
 * NOT called automatically when window is first created.
 * Called by GLFW BEFORE reshape().
*/
void
fbreshape(GLFWwindow *wd, int w, int h)
{
  /* save new framebuffer dimensions */
  fbwidth = w;
  fbheight = h;

  /* do an orthographic parallel projection with the view volume
     set to first quadrant, fixed to the initial window dimension */
  glMatrixMode(GL_PROJECTION);
  glLoadIdentity();
  glOrtho(0.0, (float)fbwidth, 0.0, (float)fbheight, -1.f, 1.f);

  /* Tell OpenGL to use the whole window for drawing.
     Note that we don't resize the view volume, so
     the viewport will show the whole view volume 
     shrunk/stretched to fit the current view port. */
  glViewport(0, 0, (GLsizei) fbwidth, (GLsizei) fbheight);

  init_data();
  
  return;
}

@ffreyer
Copy link
Collaborator Author

ffreyer commented Jan 4, 2023

Closed in favor of #2544

@ffreyer ffreyer closed this Jan 4, 2023
@ffreyer ffreyer deleted the ff/px_per_unit branch January 8, 2023 12:27
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

3 participants