Feature/light manip UI #159
Conversation
| // m_activeTool = new MoveTool(); | ||
| // m_activeTool = new RotateTool(); | ||
| // m_tools.push_back(new AreaLightTool()); | ||
| m_areaLightTool = new AreaLightTool(); |
There was a problem hiding this comment.
This code is showing two different patterns: one where I abstractly add a "disableable" tool to the list of tools (new ScaleBarTool below), and one where I explicitly create a tool and expect outside code to add/remove it as needed. I'm not sure if future work in this code will unify the two approaches, but both work for now as long as the bookkeeping is done properly.
toloudis
requested review from
meganrm,
blairlyons,
interim17,
ShrimpCryptid,
frasercl and
ascibisz
and removed request for
a team
toloudis
force-pushed
the
feature/light-manip-ui
branch
from
1634fca to
4924bb7
Compare
| // toggle rotate tool | ||
| if (m_areaLightMode == AREALIGHT_MODE::NONE || m_areaLightMode == AREALIGHT_MODE::TRANS) { | ||
| m_viewerWindow->showAreaLightGizmo(true); | ||
| m_viewerWindow->setTool( | ||
| new RotateTool(m_viewerWindow->m_toolsUseLocalSpace, ManipulationTool::s_manipulatorSize * devicePixelRatioF())); | ||
| m_viewerWindow->forEachTool( | ||
| [this](ManipulationTool* tool) { tool->setUseLocalSpace(m_viewerWindow->m_toolsUseLocalSpace); }); | ||
| m_areaLightMode = AREALIGHT_MODE::ROT; | ||
| } else { | ||
| m_viewerWindow->showAreaLightGizmo(false); | ||
| m_viewerWindow->setTool(nullptr); | ||
| m_areaLightMode = AREALIGHT_MODE::NONE; | ||
| } | ||
| } | ||
|
|
||
| // TODO currently this function is not wired up to any gui at all. | ||
| // This is because translation of area light source still needs work. | ||
| // (Currently rotation is sufficient.) | ||
| void | ||
| GLView3D::toggleAreaLightTranslateControls() | ||
| { | ||
| // toggle translate tool | ||
| if (m_areaLightMode == AREALIGHT_MODE::NONE || m_areaLightMode == AREALIGHT_MODE::ROT) { | ||
| m_viewerWindow->showAreaLightGizmo(true); | ||
| m_viewerWindow->setTool( | ||
| new MoveTool(m_viewerWindow->m_toolsUseLocalSpace, ManipulationTool::s_manipulatorSize * devicePixelRatioF())); | ||
| m_viewerWindow->forEachTool( | ||
| [this](ManipulationTool* tool) { tool->setUseLocalSpace(m_viewerWindow->m_toolsUseLocalSpace); }); | ||
| m_areaLightMode = AREALIGHT_MODE::TRANS; | ||
| } else { | ||
| m_viewerWindow->showAreaLightGizmo(false); | ||
| m_viewerWindow->setTool(nullptr); | ||
| m_areaLightMode = AREALIGHT_MODE::NONE; | ||
| } | ||
| } |
There was a problem hiding this comment.
A note for the future, there's some duplicated boilerplate between these two methods. Might be worth seeing if they can be combined or reduced whenever the translation controls go in.
There was a problem hiding this comment.
Yes, that's something for later. There's a big TODO at the top of the translate function and it may be completely changed when it's time to wire it up to something. This code is here to sort of remind us that it's similar to the rotate code but they can work together with the mode variable. Think I should just delete it?
There was a problem hiding this comment.
I think it's fine to keep in, it's a good reminder.
| if (show) { | ||
| m_tools.push_back(m_areaLightTool); | ||
| } else { | ||
| m_tools.erase(std::remove(m_tools.begin(), m_tools.end(), m_areaLightTool), m_tools.end()); |
There was a problem hiding this comment.
If there are other tools in m_tools after m_areaLightTool, would this potentially erase/hide other tools unintentionally?
There was a problem hiding this comment.
This is sort of a standard recipe for "get rid of one element from a vector, by value". std::remove finds the thing and makes sure it is shifted to the end of the list, and returns an iterator to the start of the removed things... and vector::erase drops it from m_tools.
…s-on-load add option to keep current agave settings when loading
A key missing component of in-viewport light manipulator. User interaction with the manipulator was not updating the numeric entry fields in the user interface. This change resolves that.
It also adds some code to the keypress handler so that the rotation gizmo and the light gizmo are in sync.
There is a button in the area light control panel to toggle the in-viewport controls. Or press "R" to toggle light source rotation gizmo on/off.
There is also a translation gizmo but since it is non-functional, it will stay disconnected from the UI.