Skip to content

Fix X11 cursor visibility on window enter - #100

Open
Abendlied wants to merge 1 commit into
Try:masterfrom
Abendlied:fix-cursor-visibility-clean
Open

Fix X11 cursor visibility on window enter#100
Abendlied wants to merge 1 commit into
Try:masterfrom
Abendlied:fix-cursor-visibility-clean

Conversation

@Abendlied

Copy link
Copy Markdown

Fix for Linux on X11.
The native mouse cursor remained visible in-game even though CursorShape::Hidden was requested during game startup.

The initial PR https://github.com/Try/Tempest/pull/98/changes is flawed. It adressed the symptom but violated the framework cursor management model completely.

Hi, @Abendlied and thanks for PR!

Unfortunately, in current state this is not a working code. In engine, it's allowed to call setCursorShape at any point. For example have LineEdit under the mouse and call setCursorShape on the parent window - this swap cursor incorrectly. There can be multiple windows at once, and so on.

In principle, one way to actually fix this is to reevaluate EventDispatcher::mouseOver, if widget is created or widget's geometry is changed.

Now in the new approach I try to follow the recommendation.

EventDispatcher did not know the mouse position until after receiving a MotionNotify. When the mouse was already over the game window, and no mouse movement received, the cursor state was not updated. This happened every time due to fullscreen.

The new implementation now gets the initial pointer position from EnterNotify and reevaluates the hovered widget when the window receives focus. The hover state is also reevaluated with widget geometry change.
Window::setCursorShape() stays how it is, cursor management is now in EventDispatcher.

Tested on Linux with X11:
Cursor is hidden immediately on startup of the game and remains hidden as long as OpenGothic has focus. Cursor becomes visible when losing focus, e.g. alt-tabbing. Cursor is hidden again when focus comes back to OpenGothic for hovering.

Comment thread Engine/ui/widget.cpp
w->update();
w->applyLayout();
}
SystemApi::dispatchMouseReevaluate(*implTrieRoot(this));

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Calling to reevaluate whole tree, inside Widget::setGeometry just like so, can be huge performance issue - better not touch for now.

}

void EventDispatcher::implSetMouseOver(const std::shared_ptr<Widget::Ref> &wptr,MouseEvent& orig) {
void EventDispatcher::implSetMouseOver(const std::shared_ptr<Widget::Ref> &wptr,MouseEvent& orig,bool force) {

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do you need force here?

break;
}
case EnterNotify: {
SystemApi::dispatchMouseReevaluate(cb,Point(xev.xcrossing.x,xev.xcrossing.y));

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's good call to monitor EnterWindowMask, but then it suppose to be something like SystemAPI::dispatchMouseEnter/SystemAPI::dispatchMouseLeave

Comment thread Engine/ui/widget.cpp
if(astate.disable>0)
implDisableSum(w,astate.disable);
lay->applyLayout();
SystemApi::dispatchMouseReevaluate(*implTrieRoot(this));

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same problem as in setGeometry: construction of complex UI will hammer such reevaluation

dispatchMouseReevaluate(wnd,mousePosition);
}

void EventDispatcher::dispatchMouseReevaluate(Widget& wnd, Point pos) {

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so far it look like hallucinated move-event... lets focus on EnterNotify first.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants