puglPostRedisplayRect without effect on X11

[sjaehn]

... if parameter rect.width < rect.x or rect.height < rect.y.

I did a bit more testing and it turns out that it puglPostRedisplayRect properly works with {x1, y1, x2, y2} as second parameter instead of PuglRect (= {x, y, width, height}). Did I miss something or is it a (not completed yet) API change or just a bug?

[sjaehn]

FYI, I didn't observe this problem with a 2 years old (Jan 2021) version I used before within my TK (now I try to update). I'll try to find the cause. From my first look into the recent pugl code, I can't find any cause why {x1, y1, x2, y2} works but not {x, y, width, height} as it should. In the recent version, it is AFAIK correctly passed finally to xev.xexpose.x (or y or witdth or heigth) in https://github.com/lv2/pugl/blob/bd4f79646f623e929e6aa22bea028952b515aeef/src/x11.c#L1420-L1424 to XSendEvent() in https://github.com/lv2/pugl/blob/bd4f79646f623e929e6aa22bea028952b515aeef/src/x11.c#L1478 - No idea

[sjaehn]

A short video demonstration of the problem: https://www.youtube.com/watch?v=G7f0KqphYpA

Very simple program. A window 400x300px containing a full size image (thus also 400x300px) with rolling colors. Visual content is updated in Window.cpp using puglPostRedisplayRect.

First: Full size puglPostRedisplayRect with the PuglRect parameter = 0, 0, 400, 300. Everything as expected. Second: Try to color roll only a 100x100px rect at position 200,100 with PuglRect = 200, 100, 100, 100. Nothing. Third: Use pairs of coords x1, y1, x2, y2 = 200, 100, 300, 200 instead. This works.

[drobilla]

Just a bug, it seems. While I am idly thinking about killing PuglRect entirely (nothing but trouble in most cases), it should work as it always has.

Not sure off the top of my head how this is broken, maybe the rectangle merging is wrong. I'll look into it.

[sjaehn]

I didn't see the bug inside mergeExposeEvents. Although this was my first idea too.

[drobilla]

Yeah I took a quick look and everything seems to (be trying to) do the correct thing.

Can you share the above-mentioned program's code?

[sjaehn]

The above mentioned program (and the video) was made with my toolkit which wraps around pugl. To simplify, I did the same with the naked pugl:

#include "pugl/cairo.h"
#include "pugl/pugl.h"

#include <cairo/cairo.h>

#include <math.h>
#include <stdbool.h>
#include <stdio.h>
#include <string.h>
#include <time.h>

typedef struct {
  PuglWorld*      world;
  int             quit;
  cairo_t*        cr;
  clock_t         t;
} PuglTestApp;

static void
onDisplay(PuglTestApp* app, PuglView* view, const PuglExposeEvent* event)
{
  app->cr = (cairo_t*)puglGetContext(view);
  app->t = clock();
  const double x = (double)app->t / 1000.0;
  cairo_set_source_rgba(app->cr, sin (x), sin (x + 0.667 * M_PI), sin (x + 1.333 * M_PI), 1.0);
  cairo_rectangle(app->cr, 0, 0, 400, 300);
  cairo_fill (app->cr);
}

static void
onClose(PuglView* view)
{
  PuglTestApp* app = (PuglTestApp*)puglGetHandle(view);
  app->quit = 1;
}

static PuglStatus
onEvent(PuglView* view, const PuglEvent* event)
{
  PuglTestApp* app = (PuglTestApp*)puglGetHandle(view);

  switch (event->type) {
  case PUGL_UPDATE:
    {
      PuglRect rect;
      rect.x = 150;
      rect.y = 100;
      rect.width = 200;
      rect.height = 200;
      puglPostRedisplayRect(view,rect);
    }
    break;
  case PUGL_EXPOSE:
    onDisplay(app, view, &event->expose);
    break;
  case PUGL_CLOSE:
    onClose(view);
    break;
  default:
    break;
  }

  return PUGL_SUCCESS;
}

int
main(int argc, char** argv)
{
  PuglTestApp app;
  memset(&app, 0, sizeof(app));

  app.world = puglNewWorld(PUGL_PROGRAM, 0);
  puglSetWorldString(app.world, PUGL_CLASS_NAME, "Demo");
  PuglView* view = puglNewView(app.world);
  puglSetViewString(view, PUGL_WINDOW_TITLE, "Demo");
  puglSetSizeHint(view, PUGL_DEFAULT_SIZE, 400, 300);
  puglSetViewHint(view, PUGL_RESIZABLE, PUGL_FALSE);
  puglSetHandle(view, &app);
  puglSetBackend(view, puglCairoBackend());
  puglSetEventFunc(view, onEvent);
  puglRealize(view);
  puglShow(view, PUGL_SHOW_RAISE);

  const double   timeout    = 1.0 / 60.0;
  while (!app.quit) puglUpdate(app.world, timeout);

  puglFreeView(view);
  puglFreeWorld(app.world);
  return 0;
}

Play around with the PuglRect parameters in onEvent() and you'll see it.

[drobilla]

I see. The X11 and expose dispatching mechanism works fine (easy to verify by printing the expose event bounds), but the Cairo backend screws up the clipping and/or offset so rendering partial regions doesn't work properly.

[drobilla]

Fixed in 2298803. Thanks for the test, that made the problem pretty obvious.

I added a finely-exposed mouse cursor to pugl_cairo_demo which is extremely sensitive to this sort of thing (and handily doubles as a latency tester), so hopefully this regression won't happen again.

I'm considering refining the API to truly support fine-grained exposure (so only each individual region will be exposed separately), so I'll probably tinker with that test a little more to make it very obviously broken when exposure isn't working properly. Cairo's the main case there, I think, if you're using OpenGL or Vulkan you're probably just blasting entire frames to the GPU anyway. That said, you could be smart about it there, too, so I think it's worth doing overall. For audio software, I can pretty easily imagine cases where updating a little level meter or similar would chew way less power / resources in general with this.

[sjaehn]

Cool, and your commits fixed another related (maybe the same) drawing problem I observed but not reported.

And I go with you, fractional display updating makes sense as audio software UIs become more complex and thus bigger and as CPU rendering is not dead yet. We started with plugins with just a few knobs one or two decades before. But now think about the full screen synthesizers like vitalium or surge. Or the growing LSP plugins. Or my B.Oops effect sequencer plugin. Therefore I decided to use puglPostRedisplayRect from the beginning.

Finally thanks for the quick fixing.