Skip to content

Commit 03e8493

Browse files
committed
fix UX bugs with new viewport
1 parent 43a858d commit 03e8493

File tree

6 files changed

+113
-52
lines changed

6 files changed

+113
-52
lines changed

src/appleseed.studio/mainwindow/rendering/lightpathsviewportmanager.cpp

Lines changed: 52 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,6 @@
3333
#include "mainwindow/rendering/lightpathspickinghandler.h"
3434
#include "mainwindow/rendering/lightpathslayer.h"
3535
#include "mainwindow/rendering/viewporttab.h"
36-
#include "mainwindow/rendering/viewportwidget.h"
3736
#include "utility/miscellaneous.h"
3837
#include "utility/settingskeys.h"
3938

@@ -77,9 +76,11 @@ namespace studio {
7776

7877
LightPathsViewportManager::LightPathsViewportManager(
7978
ViewportTab* viewport_tab,
80-
Project& project,
79+
Project* project,
8180
ParamArray& settings)
8281
: m_enabled(false)
82+
, m_picking_enabled(true)
83+
, m_paths_display_active(false)
8384
, m_project(project)
8485
, m_settings(settings)
8586
, m_viewport_tab(viewport_tab)
@@ -94,15 +95,49 @@ LightPathsViewportManager::LightPathsViewportManager(
9495
recreate_handlers();
9596
}
9697

98+
void LightPathsViewportManager::reset(renderer::Project* project)
99+
{
100+
set_enabled(false);
101+
set_display_enabled(false);
102+
set_picking_enabled(true);
103+
m_project = project;
104+
}
105+
106+
void LightPathsViewportManager::slot_base_layer_changed(const ViewportWidget::BaseLayer layer)
107+
{
108+
if (layer == ViewportWidget::BaseLayer::FinalRender)
109+
set_picking_enabled(true);
110+
else
111+
set_picking_enabled(false);
112+
}
113+
97114
void LightPathsViewportManager::set_enabled(const bool enabled)
98115
{
99116
m_enabled = enabled;
100117
if (enabled)
118+
{
101119
m_toolbar->show();
120+
}
102121
else
122+
{
123+
set_display_enabled(false);
103124
m_toolbar->hide();
104-
125+
}
105126
m_toolbar->setDisabled(!enabled);
127+
128+
emit signal_should_display(m_enabled && m_paths_display_active);
129+
}
130+
131+
void LightPathsViewportManager::set_display_enabled(const bool enabled)
132+
{
133+
m_paths_display_active = enabled;
134+
135+
emit signal_should_display(m_enabled && m_paths_display_active);
136+
}
137+
138+
void LightPathsViewportManager::set_picking_enabled(const bool enabled)
139+
{
140+
m_picking_enabled = enabled;
106141
m_screen_space_paths_picking_handler->set_enabled(enabled);
107142
}
108143

@@ -113,9 +148,11 @@ QToolBar* LightPathsViewportManager::toolbar() const
113148

114149
void LightPathsViewportManager::slot_entity_picked(const ScenePicker::PickingResult& result)
115150
{
116-
if (!m_enabled) return;
151+
if (!m_picking_enabled || !m_enabled) return;
117152

118-
const CanvasProperties& props = m_project.get_frame()->image().properties();
153+
set_display_enabled(true);
154+
155+
const CanvasProperties& props = m_project->get_frame()->image().properties();
119156
m_screen_space_paths_picking_handler->pick(
120157
Vector2i(
121158
result.m_ndc[0] * static_cast<int>(props.m_canvas_width),
@@ -129,8 +166,9 @@ void LightPathsViewportManager::slot_light_paths_display_toggled(const bool acti
129166

130167
void LightPathsViewportManager::slot_rectangle_selection(const QRect& rect)
131168
{
132-
if (!m_enabled) return;
169+
if (!m_picking_enabled || !m_enabled) return;
133170

171+
set_display_enabled(true);
134172
m_screen_space_paths_picking_handler->pick(
135173
AABB2i(
136174
Vector2i(rect.x(), rect.y()),
@@ -139,15 +177,17 @@ void LightPathsViewportManager::slot_rectangle_selection(const QRect& rect)
139177

140178
void LightPathsViewportManager::slot_light_path_selection_changed(
141179
const int selected_light_path_index,
142-
const int total_light_paths) const
180+
const int total_light_paths)
143181
{
144182
if (total_light_paths > 0)
145183
{
184+
set_display_enabled(true);
146185
m_prev_path_button->setEnabled(selected_light_path_index > -1);
147186
m_next_path_button->setEnabled(selected_light_path_index < total_light_paths - 1);
148187
}
149188
else
150189
{
190+
set_display_enabled(false);
151191
m_prev_path_button->setEnabled(false);
152192
m_next_path_button->setEnabled(false);
153193
}
@@ -172,7 +212,7 @@ void LightPathsViewportManager::slot_save_light_paths()
172212
filepath = QDir::toNativeSeparators(filepath);
173213

174214
// Write light paths to disk.
175-
m_project.get_light_path_recorder().write(filepath.toUtf8().constData());
215+
m_project->get_light_path_recorder().write(filepath.toUtf8().constData());
176216
}
177217

178218
void LightPathsViewportManager::slot_camera_changed()
@@ -192,7 +232,7 @@ void LightPathsViewportManager::create_toolbar()
192232
// Save Light Paths button.
193233
QToolButton* save_light_paths_button = new QToolButton();
194234
save_light_paths_button->setIcon(load_icons("lightpathstab_save_light_paths"));
195-
const auto light_path_count = m_project.get_light_path_recorder().get_light_path_count();
235+
const auto light_path_count = m_project->get_light_path_recorder().get_light_path_count();
196236
save_light_paths_button->setToolTip(
197237
QString("Save %1 Light Path%2...")
198238
.arg(QString::fromStdString(pretty_uint(light_path_count)))
@@ -233,8 +273,8 @@ void LightPathsViewportManager::create_toolbar()
233273
backface_culling_button->setCheckable(true);
234274
backface_culling_button->setChecked(false);
235275
connect(
236-
backface_culling_button, SIGNAL(toggled()),
237-
light_paths_layer, SLOT(slot_toggle_backface_culling()));
276+
backface_culling_button, SIGNAL(toggled(bool)),
277+
light_paths_layer, SLOT(slot_toggle_backface_culling(bool)));
238278
m_toolbar->addWidget(backface_culling_button);
239279

240280
// Synchronize Camera button.
@@ -268,7 +308,7 @@ void LightPathsViewportManager::recreate_handlers()
268308
new LightPathsPickingHandler(
269309
m_viewport_tab->get_viewport_widget(),
270310
*m_mouse_tracker.get(),
271-
m_project));
311+
*m_project));
272312
m_screen_space_paths_picking_handler->set_enabled(false);
273313

274314
// The world-space paths picking handler is used to pick paths in the light paths widget.

src/appleseed.studio/mainwindow/rendering/lightpathsviewportmanager.h

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232
#include "mainwindow/rendering/cameracontroller.h"
3333
#include "mainwindow/rendering/lightpathspickinghandler.h"
3434
#include "mainwindow/rendering/renderclipboardhandler.h"
35+
#include "mainwindow/rendering/viewportwidget.h"
3536
#include "utility/mousecoordinatestracker.h"
3637
#include "utility/scrollareapanhandler.h"
3738
#include "utility/widgetzoomhandler.h"
@@ -72,29 +73,39 @@ class LightPathsViewportManager
7273
public:
7374
LightPathsViewportManager(
7475
ViewportTab* viewport_tab,
75-
renderer::Project& project,
76+
renderer::Project* project,
7677
renderer::ParamArray& settings);
7778

79+
void reset(renderer::Project* project);
80+
7881
QToolBar* toolbar() const;
7982

8083
void set_enabled(const bool enabled);
84+
void set_picking_enabled(const bool enabled);
85+
void set_display_enabled(const bool enabled);
86+
87+
signals:
88+
void signal_should_display(const bool should_display);
8189

8290
public slots:
8391
void slot_entity_picked(const renderer::ScenePicker::PickingResult& result);
8492
void slot_rectangle_selection(const QRect& rect);
8593
void slot_light_paths_display_toggled(const bool active);
8694

8795
private slots:
96+
void slot_base_layer_changed(const ViewportWidget::BaseLayer layer);
8897
void slot_light_path_selection_changed(
8998
const int selected_light_path_index,
90-
const int total_light_paths) const;
99+
const int total_light_paths);
91100
void slot_save_light_paths();
92101
void slot_camera_changed();
93102

94103
private:
95104
bool m_enabled;
105+
bool m_picking_enabled;
106+
bool m_paths_display_active;
96107

97-
renderer::Project& m_project;
108+
renderer::Project* m_project;
98109
renderer::ParamArray& m_settings;
99110
ViewportTab* m_viewport_tab;
100111
QToolBar* m_toolbar;

src/appleseed.studio/mainwindow/rendering/viewporttab.cpp

Lines changed: 33 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,6 @@
3434
#include "mainwindow/project/projectexplorer.h"
3535
#include "mainwindow/rendering/lightpathsviewportmanager.h"
3636
#include "mainwindow/rendering/renderingmanager.h"
37-
#include "mainwindow/rendering/viewportwidget.h"
3837
#include "utility/miscellaneous.h"
3938

4039
// appleseed.renderer headers.
@@ -93,9 +92,9 @@ ViewportTab::ViewportTab(
9392
layout()->setMargin(0);
9493

9594
create_viewport_widget();
96-
create_light_paths_manager(m_application_settings);
9795
create_toolbar();
9896
create_scrollarea();
97+
create_light_paths_manager();
9998

10099
layout()->addWidget(m_toolbar);
101100
layout()->addWidget(m_light_paths_manager->toolbar());
@@ -134,7 +133,8 @@ void ViewportTab::render_began()
134133
{
135134
m_viewport_widget->get_render_layer()->darken();
136135
m_viewport_widget->get_light_paths_layer()->update_render_camera_transform();
137-
set_light_paths_enabled(false);
136+
m_light_paths_manager.get()->reset(&m_project);
137+
set_light_paths_toggle_enabled(false);
138138
update();
139139
}
140140

@@ -176,17 +176,8 @@ void ViewportTab::load_state(const State& state)
176176
m_pan_handler->load_state(state.m_pan_handler_state);
177177
}
178178

179-
void ViewportTab::set_light_paths_enabled(const bool enabled)
179+
void ViewportTab::slot_base_layer_changed(const ViewportWidget::BaseLayer base_layer)
180180
{
181-
m_light_paths_toggle_button->setDisabled(!enabled);
182-
m_light_paths_manager->set_enabled(enabled);
183-
}
184-
185-
void ViewportTab::slot_base_layer_changed(int index)
186-
{
187-
assert(index < ViewportWidget::BaseLayer::BASE_LAYER_MAX_VALUE);
188-
auto base_layer = static_cast<ViewportWidget::BaseLayer>(index);
189-
190181
switch (base_layer)
191182
{
192183
case ViewportWidget::BaseLayer::FinalRender:
@@ -257,12 +248,25 @@ void ViewportTab::create_viewport_widget()
257248
m_viewport_widget->setMouseTracking(true);
258249
}
259250

260-
void ViewportTab::create_light_paths_manager(renderer::ParamArray application_settings)
251+
void ViewportTab::create_light_paths_manager()
261252
{
262-
m_light_paths_manager = new LightPathsViewportManager(
253+
m_light_paths_manager.reset(new LightPathsViewportManager(
263254
this,
264-
m_project,
265-
application_settings);
255+
&m_project,
256+
m_application_settings));
257+
258+
connect(
259+
m_viewport_widget, SIGNAL(signal_base_layer_changed(const ViewportWidget::BaseLayer)),
260+
m_light_paths_manager.get(), SLOT(slot_base_layer_changed(const ViewportWidget::BaseLayer))
261+
);
262+
connect(
263+
m_light_paths_manager.get(), SIGNAL(signal_should_display(const bool)),
264+
m_viewport_widget, SLOT(slot_light_paths_should_display(const bool))
265+
);
266+
connect(
267+
m_light_paths_toggle_button, SIGNAL(toggled(bool)),
268+
m_light_paths_manager.get(), SLOT(slot_light_paths_display_toggled(bool))
269+
);
266270
}
267271

268272
void ViewportTab::create_toolbar()
@@ -289,8 +293,8 @@ void ViewportTab::create_toolbar()
289293
m_viewport_widget, SLOT(slot_base_layer_changed(int))
290294
);
291295
connect(
292-
m_base_layer_combo, SIGNAL(activated(int)),
293-
SLOT(slot_base_layer_changed(int))
296+
m_viewport_widget, SIGNAL(signal_base_layer_changed(const ViewportWidget::BaseLayer)),
297+
SLOT(slot_base_layer_changed(const ViewportWidget::BaseLayer))
294298
);
295299

296300
m_light_paths_toggle_button = new QToolButton();
@@ -548,7 +552,7 @@ void ViewportTab::recreate_handlers()
548552
m_camera_controller.get(), SLOT(slot_entity_picked(renderer::ScenePicker::PickingResult)));
549553
connect(
550554
m_scene_picking_handler.get(), SIGNAL(signal_entity_picked(renderer::ScenePicker::PickingResult)),
551-
m_light_paths_manager, SLOT(slot_entity_picked(renderer::ScenePicker::PickingResult)));
555+
m_light_paths_manager.get(), SLOT(slot_entity_picked(renderer::ScenePicker::PickingResult)));
552556

553557
// Handler for setting render regions with the mouse.
554558
m_viewport_selection_handler.reset(
@@ -560,7 +564,7 @@ void ViewportTab::recreate_handlers()
560564
SIGNAL(signal_rectangle_selection(const QRect&)));
561565
connect(
562566
m_viewport_selection_handler.get(), SIGNAL(signal_rectangle_selection(const QRect&)),
563-
m_light_paths_manager, SLOT(slot_rectangle_selection(const QRect&)));
567+
m_light_paths_manager.get(), SLOT(slot_rectangle_selection(const QRect&)));
564568
connect(
565569
m_viewport_selection_handler.get(), SIGNAL(signal_render_region(const QRect&)),
566570
SLOT(slot_set_render_region(const QRect&)));
@@ -590,6 +594,14 @@ void ViewportTab::recreate_handlers()
590594
const QString&)));
591595
}
592596

597+
void ViewportTab::set_light_paths_toggle_enabled(const bool enabled)
598+
{
599+
if (!enabled)
600+
m_light_paths_toggle_button->setChecked(false);
601+
602+
m_light_paths_toggle_button->setDisabled(!enabled);
603+
}
604+
593605
} // namespace studio
594606
} // namespace appleseed
595607

src/appleseed.studio/mainwindow/rendering/viewporttab.h

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@
3737
#include "mainwindow/rendering/renderclipboardhandler.h"
3838
#include "mainwindow/rendering/scenepickinghandler.h"
3939
#include "mainwindow/rendering/viewportregionselectionhandler.h"
40+
#include "mainwindow/rendering/viewportwidget.h"
4041
#include "utility/mousecoordinatestracker.h"
4142
#include "utility/scrollareapanhandler.h"
4243
#include "utility/widgetzoomhandler.h"
@@ -55,7 +56,6 @@ namespace OCIO = OCIO_NAMESPACE;
5556
// Forward declarations.
5657
namespace appleseed { namespace studio { class LightPathsViewportManager; } }
5758
namespace appleseed { namespace studio { class ProjectExplorer; } }
58-
namespace appleseed { namespace studio { class ViewportWidget; } }
5959
namespace renderer { class Entity; }
6060
namespace renderer { class Project; }
6161
namespace renderer { class RenderingManager; }
@@ -91,8 +91,8 @@ class ViewportTab
9191
CameraController* get_camera_controller() const;
9292
ScenePickingHandler* get_scene_picking_handler() const;
9393

94-
void set_light_paths_enabled(const bool enabled);
9594
void set_clear_frame_button_enabled(const bool enabled);
95+
void set_light_paths_toggle_enabled(const bool enabled);
9696
void set_render_region_buttons_enabled(const bool enabled);
9797

9898
void render_began();
@@ -128,15 +128,14 @@ class ViewportTab
128128

129129
private slots:
130130
void slot_camera_changed();
131-
void slot_base_layer_changed(int index);
131+
void slot_base_layer_changed(const ViewportWidget::BaseLayer base_layer);
132132
void slot_set_render_region(const QRect& rect);
133133
void slot_toggle_pixel_inspector(const bool checked);
134134
void slot_toggle_render_region(const bool checked);
135135
void slot_viewport_widget_context_menu(const QPoint& point);
136136

137137
private:
138138
ViewportWidget* m_viewport_widget;
139-
LightPathsViewportManager* m_light_paths_manager;
140139

141140
QScrollArea* m_scroll_area;
142141
QToolBar* m_toolbar;
@@ -158,6 +157,7 @@ class ViewportTab
158157
RenderingManager& m_rendering_manager;
159158
renderer::ParamArray m_application_settings;
160159

160+
std::unique_ptr<LightPathsViewportManager> m_light_paths_manager;
161161
std::unique_ptr<WidgetZoomHandler> m_zoom_handler;
162162
std::unique_ptr<ScrollAreaPanHandler> m_pan_handler;
163163
std::unique_ptr<MaterialDropHandler> m_material_drop_handler;
@@ -171,7 +171,7 @@ class ViewportTab
171171

172172
OCIO::ConstConfigRcPtr m_ocio_config;
173173

174-
void create_light_paths_manager(renderer::ParamArray application_settings);
174+
void create_light_paths_manager();
175175
void create_scrollarea();
176176
void create_toolbar();
177177
void create_viewport_widget();

0 commit comments

Comments
 (0)