From 0a827cf65c617e39a228de4406084ab5d5eba776 Mon Sep 17 00:00:00 2001 From: rdb Date: Mon, 7 Nov 2022 12:56:03 +0100 Subject: [PATCH 1/4] x11display: Time out waiting for ConfigureNotify event Fixes #1087 --- panda/src/egldisplay/eglGraphicsWindow.cxx | 2 +- panda/src/glxdisplay/glxGraphicsWindow.cxx | 2 +- panda/src/tinydisplay/tinyXGraphicsWindow.cxx | 4 +-- panda/src/x11display/x11GraphicsWindow.cxx | 28 ++++++++++++++++--- panda/src/x11display/x11GraphicsWindow.h | 2 +- 5 files changed, 29 insertions(+), 9 deletions(-) diff --git a/panda/src/egldisplay/eglGraphicsWindow.cxx b/panda/src/egldisplay/eglGraphicsWindow.cxx index 31002c9fa6..e82ea6862f 100644 --- a/panda/src/egldisplay/eglGraphicsWindow.cxx +++ b/panda/src/egldisplay/eglGraphicsWindow.cxx @@ -80,7 +80,7 @@ begin_frame(FrameMode mode, Thread *current_thread) { if (_gsg == nullptr) { return false; } - if (_awaiting_configure) { + if (_awaiting_configure_since != -1) { // Don't attempt to draw while we have just reconfigured the window and we // haven't got the notification back yet. return false; diff --git a/panda/src/glxdisplay/glxGraphicsWindow.cxx b/panda/src/glxdisplay/glxGraphicsWindow.cxx index 85757f2faa..449d1f66a0 100644 --- a/panda/src/glxdisplay/glxGraphicsWindow.cxx +++ b/panda/src/glxdisplay/glxGraphicsWindow.cxx @@ -60,7 +60,7 @@ begin_frame(FrameMode mode, Thread *current_thread) { if (_gsg == nullptr) { return false; } - if (_awaiting_configure) { + if (_awaiting_configure_since != -1) { // Don't attempt to draw while we have just reconfigured the window and we // haven't got the notification back yet. return false; diff --git a/panda/src/tinydisplay/tinyXGraphicsWindow.cxx b/panda/src/tinydisplay/tinyXGraphicsWindow.cxx index 1e71324baf..a1e83326c4 100644 --- a/panda/src/tinydisplay/tinyXGraphicsWindow.cxx +++ b/panda/src/tinydisplay/tinyXGraphicsWindow.cxx @@ -86,7 +86,7 @@ begin_frame(FrameMode mode, Thread *current_thread) { if (_gsg == nullptr) { return false; } - if (_awaiting_configure) { + if (_awaiting_configure_since != -1) { // Don't attempt to draw while we have just reconfigured the window and we // haven't got the notification back yet. return false; @@ -238,7 +238,7 @@ process_events() { break; case ConfigureNotify: - _awaiting_configure = false; + _awaiting_configure_since = -1; if (_properties.get_fixed_size()) { // If the window properties indicate a fixed size only, undo any // attempt by the user to change them. In X, there doesn't appear to diff --git a/panda/src/x11display/x11GraphicsWindow.cxx b/panda/src/x11display/x11GraphicsWindow.cxx index 8548d05eea..c8ab5e1e9c 100644 --- a/panda/src/x11display/x11GraphicsWindow.cxx +++ b/panda/src/x11display/x11GraphicsWindow.cxx @@ -115,7 +115,7 @@ x11GraphicsWindow(GraphicsEngine *engine, GraphicsPipe *pipe, _XRRSetScreenConfig = x11_pipe->_XRRSetScreenConfig; } - _awaiting_configure = false; + _awaiting_configure_since = -1; _dga_mouse_enabled = false; _override_redirect = False; _wm_delete_window = x11_pipe->_wm_delete_window; @@ -238,7 +238,7 @@ begin_frame(FrameMode mode, Thread *current_thread) { if (_gsg == nullptr) { return false; } - if (_awaiting_configure) { + if (_awaiting_configure_since != -1) { // Don't attempt to draw while we have just reconfigured the window and we // haven't got the notification back yet. return false; @@ -481,7 +481,14 @@ process_events() { if (got_configure_event) { // Now handle the last configure event we found. - _awaiting_configure = false; + if (x11display_cat.is_debug() && _awaiting_configure_since != -1) { + unsigned long elapsed = (unsigned long)(clock() - _awaiting_configure_since) / (CLOCKS_PER_SEC / 10000); + x11display_cat.debug() + << "Received ConfigureNotify event after " + << (elapsed / 10) << "." << (elapsed % 10) << " ms\n"; + } + + _awaiting_configure_since = -1; // Is this the inner corner or the outer corner? The Xlib docs say it // should be the outer corner, but it appears to be the inner corner on my @@ -523,6 +530,19 @@ process_events() { changed_properties = true; } + else if (_awaiting_configure_since != -1) { + unsigned long elapsed = (clock() - _awaiting_configure_since); + if (elapsed > CLOCKS_PER_SEC / 10) { + // Accept that we're never going to get that configure notify event. + if (x11display_cat.is_debug()) { + elapsed /= (CLOCKS_PER_SEC / 10000); + x11display_cat.debug() + << "Giving up on waiting for ConfigureNotify event after " + << (elapsed / 10) << "." << (elapsed % 10) << " ms\n"; + } + _awaiting_configure_since = -1; + } + } if (properties.has_foreground() && ( _properties.get_mouse_mode() == WindowProperties::M_confined || @@ -949,7 +969,7 @@ set_properties_now(WindowProperties &properties) { XReconfigureWMWindow(_display, _xwindow, _screen, value_mask, &changes); // Don't draw anything until this is done reconfiguring. - _awaiting_configure = true; + _awaiting_configure_since = clock(); } } diff --git a/panda/src/x11display/x11GraphicsWindow.h b/panda/src/x11display/x11GraphicsWindow.h index f512b16a19..ca44e4d426 100644 --- a/panda/src/x11display/x11GraphicsWindow.h +++ b/panda/src/x11display/x11GraphicsWindow.h @@ -90,7 +90,7 @@ protected: GraphicsWindowInputDevice *_input; long _event_mask; - bool _awaiting_configure; + clock_t _awaiting_configure_since; bool _dga_mouse_enabled; Bool _override_redirect; Atom _wm_delete_window; From 8965741fcf2616d790afbb2af4f82a004f4c5d05 Mon Sep 17 00:00:00 2001 From: rdb Date: Mon, 7 Nov 2022 12:56:56 +0100 Subject: [PATCH 2/4] tinydisplay: Don't reimplement process_events from base class Reduces code duplication, makes it possible to inherit bug fixes from the base class, and also handles resizes more efficiently now (only when this would actually require a framebuffer reallocation). --- panda/src/tinydisplay/tinyXGraphicsWindow.cxx | 187 +----------------- 1 file changed, 9 insertions(+), 178 deletions(-) diff --git a/panda/src/tinydisplay/tinyXGraphicsWindow.cxx b/panda/src/tinydisplay/tinyXGraphicsWindow.cxx index a1e83326c4..f2337536b0 100644 --- a/panda/src/tinydisplay/tinyXGraphicsWindow.cxx +++ b/panda/src/tinydisplay/tinyXGraphicsWindow.cxx @@ -187,186 +187,17 @@ supports_pixel_zoom() const { */ void TinyXGraphicsWindow:: process_events() { - LightReMutexHolder holder(TinyXGraphicsPipe::_x_mutex); + x11GraphicsWindow::process_events(); - GraphicsWindow::process_events(); + int xsize = (_properties.get_x_size() + 3) & ~3; + int ysize = _properties.get_y_size(); - if (_xwindow == (X11_Window)0) { - return; - } - - XEvent event; - XKeyEvent keyrelease_event; - bool got_keyrelease_event = false; - - while (XCheckIfEvent(_display, &event, check_event, (char *)this)) { - if (XFilterEvent(&event, None)) { - continue; - } - - if (got_keyrelease_event) { - // If a keyrelease event is immediately followed by a matching keypress - // event, that's just key repeat and we should treat the two events - // accordingly. It would be nice if X provided a way to differentiate - // between keyrepeat and explicit keypresses more generally. - got_keyrelease_event = false; - - if (event.type == KeyPress && - event.xkey.keycode == keyrelease_event.keycode && - (event.xkey.time - keyrelease_event.time <= 1)) { - // In particular, we only generate down messages for the repeated - // keys, not down-and-up messages. - handle_keystroke(event.xkey); - - // We thought about not generating the keypress event, but we need - // that repeat for backspace. Rethink later. - handle_keypress(event.xkey); - continue; - - } else { - // This keyrelease event is not immediately followed by a matching - // keypress event, so it's a genuine release. - handle_keyrelease(keyrelease_event); - } - } - - WindowProperties properties; - ButtonHandle button; - - switch (event.type) { - case ReparentNotify: - break; - - case ConfigureNotify: - _awaiting_configure_since = -1; - if (_properties.get_fixed_size()) { - // If the window properties indicate a fixed size only, undo any - // attempt by the user to change them. In X, there doesn't appear to - // be a way to universally disallow this directly (although we do set - // the min_size and max_size to the same value, which seems to work - // for most window managers.) - WindowProperties current_props = get_properties(); - if (event.xconfigure.width != current_props.get_x_size() || - event.xconfigure.height != current_props.get_y_size()) { - XWindowChanges changes; - changes.width = current_props.get_x_size(); - changes.height = current_props.get_y_size(); - int value_mask = (CWWidth | CWHeight); - XConfigureWindow(_display, _xwindow, value_mask, &changes); - } - - } else { - // A normal window may be resized by the user at will. - properties.set_size(event.xconfigure.width, event.xconfigure.height); - system_changed_properties(properties); - ZB_resize(_full_frame_buffer, nullptr, _properties.get_x_size(), _properties.get_y_size()); - _pitch = (_full_frame_buffer->xsize * _bytes_per_pixel + 3) & ~3; - create_reduced_frame_buffer(); - create_ximage(); - } - break; - - case ButtonPress: - // This refers to the mouse buttons. - button = get_mouse_button(event.xbutton); - _input->set_pointer_in_window(event.xbutton.x, event.xbutton.y); - _input->button_down(button); - break; - - case ButtonRelease: - button = get_mouse_button(event.xbutton); - _input->set_pointer_in_window(event.xbutton.x, event.xbutton.y); - _input->button_up(button); - break; - - case MotionNotify: - _input->set_pointer_in_window(event.xmotion.x, event.xmotion.y); - break; - - case KeyPress: - handle_keystroke(event.xkey); - handle_keypress(event.xkey); - break; - - case KeyRelease: - // The KeyRelease can't be processed immediately, because we have to - // check first if it's immediately followed by a matching KeyPress - // event. - keyrelease_event = event.xkey; - got_keyrelease_event = true; - break; - - case EnterNotify: - _input->set_pointer_in_window(event.xcrossing.x, event.xcrossing.y); - break; - - case LeaveNotify: - _input->set_pointer_out_of_window(); - break; - - case FocusIn: - properties.set_foreground(true); - system_changed_properties(properties); - break; - - case FocusOut: - _input->focus_lost(); - properties.set_foreground(false); - system_changed_properties(properties); - break; - - case UnmapNotify: - properties.set_minimized(true); - system_changed_properties(properties); - break; - - case MapNotify: - properties.set_minimized(false); - system_changed_properties(properties); - - // Auto-focus the window when it is mapped. - XSetInputFocus(_display, _xwindow, RevertToPointerRoot, CurrentTime); - break; - - case ClientMessage: - if ((Atom)(event.xclient.data.l[0]) == _wm_delete_window) { - // This is a message from the window manager indicating that the user - // has requested to close the window. - std::string close_request_event = get_close_request_event(); - if (!close_request_event.empty()) { - // In this case, the app has indicated a desire to intercept the - // request and process it directly. - throw_event(close_request_event); - - } else { - // In this case, the default case, the app does not intend to - // service the request, so we do by closing the window. - - // TODO: don't release the gsg in the window thread. - close_window(); - properties.set_open(false); - system_changed_properties(properties); - } - } - break; - - case DestroyNotify: - // Apparently, we never get a DestroyNotify on a toplevel window. - // Instead, we rely on hints from the window manager (see above). - tinydisplay_cat.info() - << "DestroyNotify\n"; - break; - - default: - tinydisplay_cat.error() - << "unhandled X event type " << event.type << "\n"; - } - } - - if (got_keyrelease_event) { - // This keyrelease event is not immediately followed by a matching - // keypress event, so it's a genuine release. - handle_keyrelease(keyrelease_event); + if (xsize != _full_frame_buffer->xsize || + ysize != _full_frame_buffer->ysize) { + ZB_resize(_full_frame_buffer, nullptr, xsize, ysize); + _pitch = (_full_frame_buffer->xsize * _bytes_per_pixel + 3) & ~3; + create_reduced_frame_buffer(); + create_ximage(); } } From 40618e104d096a7cceb4077fcb614253b4ff4cd4 Mon Sep 17 00:00:00 2001 From: "Erica M. (\"Loonatic\")" Date: Sat, 29 Oct 2022 19:41:18 -0500 Subject: [PATCH 3/4] chore: Remove extraneous print statement There's no reason why this should be printed out by default. It can create a lot of spam in the output when utilizing this module. Fixes #1383 --- direct/src/showutil/Effects.py | 1 - 1 file changed, 1 deletion(-) diff --git a/direct/src/showutil/Effects.py b/direct/src/showutil/Effects.py index d98223711b..7c21315ddf 100644 --- a/direct/src/showutil/Effects.py +++ b/direct/src/showutil/Effects.py @@ -105,7 +105,6 @@ def createBounce(nodeObj, numBounces, startValues, totalTime, amplitude, newVec3 = Vec3(startValues) newVec3.setCell(index, currBounceVal) - print("### newVec3 = %s" % newVec3) # create the right type of lerp if ((bounceType == SX_BOUNCE) or (bounceType == SY_BOUNCE) or From 11746457ff31f373ff7ae25adb365f334443230f Mon Sep 17 00:00:00 2001 From: rdb Date: Mon, 7 Nov 2022 18:02:43 +0100 Subject: [PATCH 4/4] interrogate: Fix invalid syntax for `make()` coerce ctor with kwargs [skip ci] --- dtool/src/interrogate/interfaceMakerPythonNative.cxx | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/dtool/src/interrogate/interfaceMakerPythonNative.cxx b/dtool/src/interrogate/interfaceMakerPythonNative.cxx index a767b62309..eb930185e6 100644 --- a/dtool/src/interrogate/interfaceMakerPythonNative.cxx +++ b/dtool/src/interrogate/interfaceMakerPythonNative.cxx @@ -4707,9 +4707,13 @@ write_function_instance(ostream &out, FunctionRemap *remap, // The function handles the arguments by itself. expected_params += "*args"; pexprs.push_back("args"); - if (args_type == AT_keyword_args) { - expected_params += ", **kwargs"; - pexprs.push_back("kwds"); + if (remap->_args_type == AT_keyword_args) { + if (args_type == AT_keyword_args) { + expected_params += ", **kwargs"; + pexprs.push_back("kwds"); + } else { + pexprs.push_back("nullptr"); + } } num_params = 0; }