Add secondary viewport #32

Troy-Boy · 2022-03-21T18:59:51Z

draft PR for reviewing

CHrlS98

Two lines caught my attention. Might be the source of some problems.

Éventuellement enlève les tabs que tu as ajouté dans application.h/.cpp aussi, stp.

Engine/src/application.cpp

CHrlS98 · 2022-03-24T00:24:16Z

Super cool! Some comments on the behaviour.

The secondary camera does not scale on window resize (the aspect ratio is wrong and the image appears stretched out).
The slider does not work properly for scaling the window. The arrows do the job but the slider always gets back to its previous position when the mouse button is released.
However, since there are only 4 options maybe it'd look better if we used radio buttons for the subwindow size? We could label them 1/2, 1/3, 1/4 and 1/5.

CHrlS98

Très cool! Rien de majeur, merge upstream aussi.

.gitignore

Engine/include/application.h

Engine/src/application.cpp

Engine/src/gui.cpp

CHrlS98

Cool, beau code clean. qqes trucs de formattage, ça achève 👍

CHrlS98 · 2022-03-31T15:22:43Z

Engine/src/application.cpp

-const std::string ICON32_FNAME = "/icons/icon32.png";
-const std::string ICON48_FNAME = "/icons/icon48.png";
-const std::string ICON64_FNAME = "/icons/icon64.png";
+    const unsigned int WIN_WIDTH = 800;


Still looks indented

Engine/src/application.cpp

Engine/include/application.h

CHrlS98 · 2022-03-31T15:27:24Z

Engine/include/camera.h

@@ -50,7 +58,7 @@ class Camera
        glm::mat4 viewMatrix;
        glm::mat4 projectionMatrix;
    };
-
+    


whitespaces are back

Engine/src/application.cpp

Engine/src/camera.cpp

Engine/src/application.cpp

CHrlS98 · 2022-03-31T15:55:27Z

Still deformed, try resizing the secondary camera even when magnifying mode is not enabled, to always be up to date. (application.cpp line 312) Also it looks like you use app->magnifyingMode when you should use applicationState-> MagnifyingMode?

Engine/src/camera.cpp

CHrlS98 · 2022-04-04T01:08:52Z

Engine/src/camera.cpp

+    mProjectionMatrix = camera.mProjectionMatrix;
+    mViewMatrix = camera.mViewMatrix;
+    mBlockRotation = camera.mBlockRotation;
+    mCamParamsData = camera.mCamParamsData;


Do we really need to copy this one? It's the GPU binding and I think it should be unique to each instance.

Or at least not modified by this method.

Engine/src/application.cpp

CHrlS98 · 2022-04-04T01:46:42Z

I'd like to suggest a change for adding a border to the secondary viewport. Here is how to do it:
First, in the anonymous namespace of application.cpp, add

const int SECONDARY_VIEWPORT_BORDER_WIDTH = 2;

Then in application.cpp, replace line 183 by this code snippet:

glClearColor(1.0f, 1.0f, 1.0f, 1.0f);
glClear(GL_COLOR_BUFFER_BIT);
glViewport(SECONDARY_VIEWPORT_BORDER_WIDTH,
           SECONDARY_VIEWPORT_BORDER_WIDTH,
           w / scaleFactor - 2 * SECONDARY_VIEWPORT_BORDER_WIDTH,
           h / scaleFactor - 2 * SECONDARY_VIEWPORT_BORDER_WIDTH);
glScissor(SECONDARY_VIEWPORT_BORDER_WIDTH,
          SECONDARY_VIEWPORT_BORDER_WIDTH,
          w / scaleFactor - 2 * SECONDARY_VIEWPORT_BORDER_WIDTH,
          h / scaleFactor - 2 * SECONDARY_VIEWPORT_BORDER_WIDTH);
glClearColor(0.0f, 0.0f, 0.0f, 0.0f);
mScene->Render();

Basically what this does is fill the secondary viewport with white and then render the scene in a slightly smaller viewport inside the secondary viewport.

Also can you default to 1/3 I feel like 1/4 is too small.

It would give a visual like this :

Troy-Boy · 2022-04-05T15:54:55Z

setter à 1/3
bordure blanche
tout est bien formater (je crois)

* added gl scissor test * added new data to gitignore * changer l'endroit de la fonction d'affichage du viewport * viewport control is good, missing render * viewport added * viewport control working * gotta move the camera boy * added camera rotation * almost there * code cleaning * camera rotation done * alias * main camera zoom too idk y * camera controls are ok * fixed camera synchronization * removed auto formatter tabs * translation fixed * add magnifying mode in gui * fix window resize * most minor fixes for pr * final pr fixes * delete unused fct * 2d mode switched to camera * changed 2d mode to camera * finished merging * formatting and camera reset fct * added border to secondary vp * finished Co-authored-by: Achille Lanctôt-Saumure <lana2914@dinf-0051-28b.DInf.fsci.usherbrooke.ca> Co-authored-by: lana2914 <achille.lanctot-saumure@usherbrooke.ca>

CHrlS98 marked this pull request as draft March 21, 2022 19:03

CHrlS98 changed the title ~~Add viewport~~ [WIP] Add secondary viewport Mar 21, 2022

CHrlS98 reviewed Mar 23, 2022

View reviewed changes

Engine/src/application.cpp Outdated Show resolved Hide resolved

Engine/src/application.cpp Outdated Show resolved Hide resolved

CHrlS98 reviewed Mar 24, 2022

View reviewed changes

CHrlS98 marked this pull request as ready for review March 24, 2022 13:55

CHrlS98 changed the title ~~[WIP] Add secondary viewport~~ Add secondary viewport Mar 24, 2022

Troy-Boy force-pushed the addViewport branch from fc0981b to 16f003a Compare March 30, 2022 18:11

CHrlS98 reviewed Mar 31, 2022

View reviewed changes

Achille Lanctôt-Saumure and others added 20 commits April 3, 2022 16:53

added gl scissor test

ac86ee1

added new data to gitignore

8336b80

changer l'endroit de la fonction d'affichage du viewport

0a0cfad

viewport control is good, missing render

6df04a0

viewport added

f7a728e

viewport control working

944a3ff

gotta move the camera boy

9de68f5

added camera rotation

ec516f2

almost there

8045971

code cleaning

25261b2

camera rotation done

86dd398

alias

d6e8d08

main camera zoom too idk y

0aeeba5

camera controls are ok

886e24b

fixed camera synchronization

0fbca71

removed auto formatter tabs

027bb08

translation fixed

70097a4

add magnifying mode in gui

64a2bee

fix window resize

0c2f249

most minor fixes for pr

9d9d92c

Achille Lanctôt-Saumure and others added 6 commits April 3, 2022 16:53

final pr fixes

a2014df

delete unused fct

e707582

2d mode switched to camera

8dac86f

changed 2d mode to camera

34bcb61

finished merging

7553a45

formatting and camera reset fct

fa3088b

Troy-Boy force-pushed the addViewport branch from f3a0e23 to fa3088b Compare April 3, 2022 20:57

Troy-Boy requested a review from CHrlS98 April 3, 2022 20:58

CHrlS98 reviewed Apr 4, 2022

View reviewed changes

added border to secondary vp

01c9460

Troy-Boy requested a review from CHrlS98 April 5, 2022 15:24

finished

5d59a68

CHrlS98 approved these changes Apr 5, 2022

View reviewed changes

CHrlS98 merged commit 08028c9 into scilus:main Apr 5, 2022

Troy-Boy deleted the addViewport branch April 5, 2022 16:50

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add secondary viewport #32

Add secondary viewport #32

Troy-Boy commented Mar 21, 2022

CHrlS98 left a comment

CHrlS98 commented Mar 24, 2022

CHrlS98 left a comment

CHrlS98 left a comment

CHrlS98 Mar 31, 2022

CHrlS98 Mar 31, 2022

CHrlS98 commented Mar 31, 2022

CHrlS98 Apr 4, 2022

CHrlS98 Apr 4, 2022

CHrlS98 commented Apr 4, 2022

Troy-Boy commented Apr 5, 2022

Add secondary viewport #32

Add secondary viewport #32

Conversation

Troy-Boy commented Mar 21, 2022

CHrlS98 left a comment

Choose a reason for hiding this comment

CHrlS98 commented Mar 24, 2022

CHrlS98 left a comment

Choose a reason for hiding this comment

CHrlS98 left a comment

Choose a reason for hiding this comment

CHrlS98 Mar 31, 2022

Choose a reason for hiding this comment

CHrlS98 Mar 31, 2022

Choose a reason for hiding this comment

CHrlS98 commented Mar 31, 2022

CHrlS98 Apr 4, 2022

Choose a reason for hiding this comment

CHrlS98 Apr 4, 2022

Choose a reason for hiding this comment

CHrlS98 commented Apr 4, 2022

Troy-Boy commented Apr 5, 2022