Opened 3 months ago

Closed 4 days ago

#1167 closed Bug Report (fixed)

Pugl does not emit the expose event when switching virtual desktops

Reported by: jpcima Owned by: dave
Priority: major Component: Pugl
Keywords: Cc:

Description

For LV2 programming convenience I am currently developing the project skeleton.lv2, a plugin template which can be found under the github user jpcima.

The default GUI is based on Pugl and the idle interface of LV2.
I use the expose events to redraw the screen when the window shows on screen.
However, there is in one particular condition Pugl in which does not retransmit the expose event from X11.

I reproduce the condition this way:

  • suppose having two virtual desktops (1) (2)
  • on desktop (1), some arbitrary window (a)
  • on desktop (2), some arbitrary window (b) and our pugl window (c)

Set focus on window (b), then switch to (1) and back to (2).
No expose event arrives and the contents of the view remains garbage.

I have traced the problem in Pugl and I have found, in puglDispatchEvent, pugl_internal.h:

if (event->expose.count == 0) {
	puglEnterContext(view);
	view->eventFunc(view, event);
	puglLeaveContext(view, true);
}

This is the condition blocking pugl from emitting the needed event.
Once commented, the program behaves the way I expect.
What purpose does this check serve? is it a good idea to remove completely?

Before digging into pugl, I have worked around this issue using the following "fix"
https://github.com/jpcima/skeleton.lv2/commit/42f4fdd7fb2942a0ce19b5580ed6b8f4200c3804.
Being no GL expert I have some doubt in the correctness of the said fix, it results in forcing a GL flush/buffer swap every idle callback. The effect is approximately +1% CPU load but I would rather avoid this overhead if possible.

Attachments (3)

cap2.jpg (177.2 KB) - added by jpcima 3 months ago.
expose.diff (591 bytes) - added by jpcima 3 months ago.
expose2.diff (2.2 KB) - added by jpcima 3 months ago.

Download all attachments as: .zip

Change History (11)

comment:1 Changed 3 months ago by jpcima

  • Component changed from BLOP to Pugl

comment:2 Changed 3 months ago by dave

Hm. The count field comes directly from X, its purpose is to notify the application that more expose events are to follow so apps that redraw the entire window can avoid redundant redisplays.

https://tronche.com/gui/x/xlib/events/exposure/expose.html

Can you reproduce this with pugl_test? It works fine for me, e.g. switching back to the desktop with pugl_test on it results in an expose. Maybe the problem is in the embedding.

comment:3 Changed 3 months ago by jpcima

I confirm using pugl_test what I said in the report.
The basic program does not print a message on expose and configure event, so I took the liberty to add those.
Switching back and forth does not result in an Expose event, and the view is garbage.

I will attach a screen capture. Admittedly the WM I'm using is not the most common of the basket, so I've suspected that maybe it was the fault of the WM I don't know. I will repeat the test in another.

Changed 3 months ago by jpcima

comment:4 Changed 3 months ago by jpcima

I tested on MATE desktop and Expose works there. So I guess indeed, the error has to be the fault of fvwm.

comment:5 Changed 3 months ago by jpcima

I've made the new observation that the condition would only happen if desktop (2) windows were in overlap, no problem otherwise. MATE would work correctly in either case.
Anyway, this appears just to be some WM weirdness and this report should probably be closed.

comment:6 Changed 3 months ago by jpcima

Ok I take back what I have said above. I believe I have figured out what is going wrong.
Pugl merges together the consecutive expose events coming from X11, and by doing so, it overwrites the count with the superior value.
Patch attached fixes it.

Changed 3 months ago by jpcima

comment:7 Changed 3 months ago by jpcima

After some more thinking about it, I think why the solution I have given is not ideal.

The count would be best updated inside the function merge_draw_events, where it is checked if a former event was encountered. The previous patch omitted to check and it was a mistake.

Secondly, I want to make corrections on the merge function and its use.
I see this function serves for both Expose and Configure events, because the start of both structures are isomorphic.

  1. It takes two events and computes a union of the rectangles, as I understand it. Good for Expose events but not so for Configure I think, because it is the latest information that is of interest (window position and size), and not the combination.
  1. The computation of the union is not correct. The width/height of the result rectangle should be deduced from the max of bottom right corners. I did not test correctness of this. (sick and tired here atm, but I hopefully got this right)

Is this new patch good to merge?

Changed 3 months ago by jpcima

comment:8 Changed 4 days ago by dave

  • Resolution set to fixed
  • Status changed from new to closed

Seems reasonable to me. The count should probably just be exactly the count of the later event, but this should be equivalent to the min of both as you have done here.

Good catch on the rectangle union.

A version of these changes rewritten in a slightly different way is in 8b8f97d/pugl, thanks.

Note: See TracTickets for help on using tickets.