magnum-extras icon indicating copy to clipboard operation
magnum-extras copied to clipboard

Use Matrix4 for viewProjection to allow for 3D UI

Open Squareys opened this issue 6 years ago • 4 comments

Hi @mosra !

As discussed, I changed all the Matrix3 projection matrices to Matrix4 to allow for arbitrary transforms of UserInterfaces. I also added AbstractUserInterface::setViewProjectionMatrix. We should question whether this is the right approach. As far as I see, this does not allow separating planes from each other yet, hence, having a single UserInterface with multiple panels with their own 3D transformations each. Will look into that soon, though.

Thanks in advance!

Cheers, Jonathan.

Squareys avatar May 10 '18 21:05 Squareys

Codecov Report

Merging #3 into master will not change coverage. The diff coverage is 14.28%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master       #3   +/-   ##
=======================================
  Coverage   19.24%   19.24%           
=======================================
  Files          35       35           
  Lines        1408     1408           
=======================================
  Hits          271      271           
  Misses       1137     1137
Impacted Files Coverage Δ
src/Magnum/Ui/BasicUserInterface.h 100% <ø> (ø) :arrow_up:
src/Magnum/Ui/BasicPlane.hpp 21.05% <0%> (ø) :arrow_up:
src/Magnum/Ui/AbstractUiShader.h 0% <0%> (ø) :arrow_up:
src/Magnum/Ui/BasicPlane.h 91.66% <0%> (ø) :arrow_up:
src/Magnum/Ui/BasicUserInterface.hpp 10% <0%> (ø) :arrow_up:
src/Magnum/Ui/BasicUserInterface.cpp 23.33% <100%> (ø) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update abed046...c34afed. Read the comment docs.

codecov-io avatar May 10 '18 22:05 codecov-io

Some thoughts on this:

  • the Ui::Plane only purpose was to organize things for a correct back-to-front rendering, i never thought about anything else (until now!)
  • your proposal for 3D rendering (each plane on a different location in 3D) could be also extended for rendering to different windows in 2D, for example
  • so maybe it would make more sense to have the projection matrix per-plane instead of per-ui?
  • did you measure perf difference compared to using mat3? i wonder how much impact it has

mosra avatar May 11 '18 11:05 mosra

could be also extended for rendering to different windows in 2D, for example

Yes! Different locations is basically different windows, good point.

projection matrix per-plane instead of per-ui?

I think view projection matrix makes sense per UI. But there needs to be a per-Window transformation maybe. I don't know if different projections for different windows makes sense... In the 2D use case, maybe, because they have different render targets (right?), but for 3D less so 🤔

did you measure perf difference compared to using mat3?

No... should I write a benchmark? Maybe a good thing to have that in general...

Squareys avatar May 11 '18 11:05 Squareys

I think view projection matrix makes sense per UI. But there needs to be a per-Window transformation maybe. I don't know if different projections for different windows makes sense... In the 2D use case, maybe, because they have different render targets (right?), but for 3D less so thinking

No, I mean: with this you could allow separating planes from each other to position them in arbitrary locations in the 3D environment, as you said above:

having a single UserInterface with multiple panels with their own 3D transformations each.

should I write a benchmark?

Since there's only a possibility to have just the mat3 or the mat4 implementation but not both, I'm not sure if that would work. I meant rather checking this to be sure it doesn't add a significant perf drop for 2D UIs (and think about this a bit more if it actually does). You could use that VR performance HUD or some other simple mean to verify the difference, no need to have exact numbers.

mosra avatar May 11 '18 14:05 mosra