Fix context menu handling in multi-viewport scenarios (the logic bugs here were many and nuanced, but we're narrowing in on something robust).
Specifically this: -Ensures key/mouse up event propagation works across multiple viewports -Ensures that mouse up events for manipulators only get delivered if there's a corresponding mouse down event -Also tidies up the "are we done processing events this tick?" logic in ViewportManipulatorController
This commit is contained in:
@@ -95,6 +95,11 @@ bool ViewportManipulatorControllerInstance::HandleInputChannelEvent(const AzFram
|
||||
AZStd::optional<MouseButton> overrideButton;
|
||||
AZStd::optional<MouseEvent> eventType;
|
||||
|
||||
// Because we receive events multiple times at separate priorities for manipulator events and
|
||||
// viewport interaction events, we want to avoid updating our "last tick state" until we're on our last event,
|
||||
// which currently is the low priority Interaction processor.
|
||||
const bool finishedProcessingEvents = event.m_priority == InteractionPriority;
|
||||
|
||||
if (IsMouseMove(event.m_inputChannel))
|
||||
{
|
||||
// Cache the ray trace results when doing manipulator interaction checks, no need to recalculate after
|
||||
@@ -120,10 +125,11 @@ bool ViewportManipulatorControllerInstance::HandleInputChannelEvent(const AzFram
|
||||
}
|
||||
else if (auto mouseButton = GetMouseButton(event.m_inputChannel); mouseButton != MouseButton::None)
|
||||
{
|
||||
const AZ::u32 mouseButtonValue = static_cast<AZ::u32>(mouseButton);
|
||||
overrideButton = mouseButton;
|
||||
if (event.m_inputChannel.GetState() == InputChannel::State::Began)
|
||||
{
|
||||
m_state.m_mouseButtons.m_mouseButtons |= static_cast<AZ::u32>(mouseButton);
|
||||
m_state.m_mouseButtons.m_mouseButtons |= mouseButtonValue;
|
||||
if (IsDoubleClick(mouseButton))
|
||||
{
|
||||
// Only remove the double click flag once we're done processing both Manipulator and Interaction events
|
||||
@@ -135,8 +141,8 @@ bool ViewportManipulatorControllerInstance::HandleInputChannelEvent(const AzFram
|
||||
}
|
||||
else
|
||||
{
|
||||
// Only insert the double click timing once we're done processing both Manipulator and Interaction events, to avoid a false IsDoubleClick positive
|
||||
if (event.m_priority == InteractionPriority)
|
||||
// Only insert the double click timing once we're done processing events, to avoid a false IsDoubleClick positive
|
||||
if (finishedProcessingEvents)
|
||||
{
|
||||
m_pendingDoubleClicks[mouseButton] = m_curTime;
|
||||
}
|
||||
@@ -145,8 +151,18 @@ bool ViewportManipulatorControllerInstance::HandleInputChannelEvent(const AzFram
|
||||
}
|
||||
else if (event.m_inputChannel.GetState() == InputChannel::State::Ended)
|
||||
{
|
||||
m_state.m_mouseButtons.m_mouseButtons &= ~static_cast<AZ::u32>(mouseButton);
|
||||
eventType = MouseEvent::Up;
|
||||
// If we've actually logged a mouse down event, forward a mouse up event.
|
||||
// This prevents corner cases like the context menu thinking it should be opened even though no one clicked in this viewport,
|
||||
// due to RenderViewportWidget ensuring all controllers get InputChannel::State::Ended events.
|
||||
if (m_state.m_mouseButtons.m_mouseButtons & mouseButtonValue)
|
||||
{
|
||||
// Erase the button from our state if we're done processing events.
|
||||
if (event.m_priority == InteractionPriority)
|
||||
{
|
||||
m_state.m_mouseButtons.m_mouseButtons &= ~mouseButtonValue;
|
||||
}
|
||||
eventType = MouseEvent::Up;
|
||||
}
|
||||
}
|
||||
}
|
||||
else if (auto keyboardModifier = GetKeyboardModifier(event.m_inputChannel); keyboardModifier != KeyboardModifier::None)
|
||||
|
||||
@@ -164,6 +164,8 @@ namespace AtomToolsFramework
|
||||
|
||||
bool RenderViewportWidget::OnInputChannelEventFiltered(const AzFramework::InputChannel& inputChannel)
|
||||
{
|
||||
bool shouldConsumeEvent = true;
|
||||
|
||||
// Grab keyboard focus if we've been clicked on.
|
||||
// Qt normally handles this for us, but we're filtering native events before they get
|
||||
// synthesized into QMouseEvents.
|
||||
@@ -175,9 +177,18 @@ namespace AtomToolsFramework
|
||||
// Don't consume new input events if we don't currently have focus.
|
||||
// We do forward Ended events, as they may be relevant to our current state
|
||||
// (e.g. a key gets released after we lose focus, it shouldn't remain "stuck").
|
||||
if (!hasFocus() && inputChannel.GetState() != AzFramework::InputChannel::State::Ended)
|
||||
if (!hasFocus())
|
||||
{
|
||||
return false;
|
||||
if (inputChannel.GetState() == AzFramework::InputChannel::State::Ended)
|
||||
{
|
||||
// Forward the input ended event to our controllers, but don't prevent other viewports from receiving it.
|
||||
shouldConsumeEvent = false;
|
||||
}
|
||||
else
|
||||
{
|
||||
// Not an event we should listen to, abort
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
// If we receive a mouse button event from outside of our viewport, ignore it even if we have focus.
|
||||
@@ -196,7 +207,9 @@ namespace AtomToolsFramework
|
||||
}
|
||||
|
||||
AzFramework::NativeWindowHandle windowId = reinterpret_cast<AzFramework::NativeWindowHandle>(winId());
|
||||
return m_controllerList->HandleInputChannelEvent({GetId(), windowId, inputChannel});
|
||||
const bool eventHandled = m_controllerList->HandleInputChannelEvent({GetId(), windowId, inputChannel});
|
||||
// If our controllers handled the event and it's one we can safely consume (i.e. it's not an Ended event that other viewports might need), consume it.
|
||||
return eventHandled && shouldConsumeEvent;
|
||||
}
|
||||
|
||||
void RenderViewportWidget::OnTick([[maybe_unused]]float deltaTime, AZ::ScriptTimePoint time)
|
||||
|
||||
Reference in New Issue
Block a user