| Summary: | DamageSubtract is misdesigned, making DeltaRectangles mode unusable | ||
|---|---|---|---|
| Product: | xorg | Reporter: | Nathaniel Smith <njs> |
| Component: | Server/General | Assignee: | Keith Packard <keithp> |
| Status: | RESOLVED MOVED | QA Contact: | Xorg Project Team <xorg-team> |
| Severity: | normal | ||
| Priority: | medium | CC: | keith.kriewall |
| Version: | 7.3 (2007.09) | ||
| Hardware: | Other | ||
| OS: | All | ||
| Whiteboard: | 2011BRB_Reviewed | ||
| i915 platform: | i915 features: | ||
| Bug Depends on: | |||
| Bug Blocks: | 44202 | ||
given a complete lack of activity, this is far too late for 7.5, even though it sounds useful to fix for 7.6. keithp? Yeah, my bad, real life intervened and I haven't had a chance to follow up properly (and that may be true for a while yet). In any case patch itself is pretty trivial, it's just the policy decision on what exactly it should do... I don't think this is a bug -- you should process all of the rectangles in a single operation (using a region). That's how this is supposed to work. (In reply to comment #3) > I don't think this is a bug -- you should process all of the rectangles in a > single operation (using a region). That's how this is supposed to work. Quoting the original report: "[Existing uses of DamageSubtract] fall into two classes: -- traditional cm's, which use NonEmpty and set repair=None, so they take branch A and B.3 is irrelevant. -- apps that want to keep track of what's on the screen (like xpra, and byzanz, and vino), which use DeltaRectangles and are bitten by this bug. " What you say is fine for the first class, but I don't see how it works for the second class. AFAIK, there *isn't* any "single operation" that lets you suck the contents of a region out of the X server (well, short of sucking down the whole bounding box). I have to handle the rectangles client-side, because unlike a traditional compositing manager, I'm not moving the rendered contents around inside the server -- I'm pulling out of the server and stuffing it down a network pipe. I can't stuff everything down the network pipe at the same time (at least, without degrading quality-of-service) because if I buffer too much then the screen contents may change before my copy of it even hits the wire, and latency will be reduced. It seems to me that this is exactly the sort of case that DamageReportDeltaRectangles is designed for, but this feature (which is itself apparently useless?) totally cripples DeltaRectangles mode. It might be that there's another way to solve such problems within the existing X server, but since AFAICT there isn't, re-opening. (If you *do* know of a solution, there are lots of programs that would benefit from you sharing your knowledge!) Too risky to change in 1.12 convergence. Moving to 1.13 tracker. (In reply to comment #4) > (In reply to comment #3) > > I don't think this is a bug -- you should process all of the rectangles in a > > single operation (using a region). That's how this is supposed to work. > > Quoting the original report: "[Existing uses of DamageSubtract] fall into two > classes: > -- traditional cm's, which use NonEmpty and set repair=None, so they take > branch A and B.3 is irrelevant. > -- apps that want to keep track of what's on the screen (like xpra, and > byzanz, and vino), which use DeltaRectangles and are bitten by this bug. > " > > What you say is fine for the first class, but I don't see how it works for the > second class. AFAIK, there *isn't* any "single operation" that lets you suck > the contents of a region out of the X server (well, short of sucking down the > whole bounding box). Is XFixesFetchRegion not this? I agree though, the re-notify behaviour is really bizarre, and I can imagine wanting to avoid it. For that matter I'm not sure why it's there in the first place. I'll see what I can do about this, I've got some other Damage changes I want to see anyway. (In reply to comment #6) > (In reply to comment #4) > > What you say is fine for the first class, but I don't see how it works for the > > second class. AFAIK, there *isn't* any "single operation" that lets you suck > > the contents of a region out of the X server (well, short of sucking down the > > whole bounding box). > > Is XFixesFetchRegion not this? XFixesFetchRegion lets us find out which rectangles on the screen are included in a region. What I'm talking about is copying the *contents* of those rectangles out into a client. Anyway, it doesn't really matter, because I only want to handle a bit of the screen at any one time anyway, so as to minimize latency... The use case here is things like tools for screen recording, screen casting, remote display, etc., which need to keep a client-side record of what's being shown on the screen. > I agree though, the re-notify behaviour is really bizarre, and I can imagine > wanting to avoid it. For that matter I'm not sure why it's there in the first > place. Yes, I can't think of any way to make use of it either. > I'll see what I can do about this, I've got some other Damage changes I want to > see anyway. That would be excellent! I don't think anyone would complain if you just diked it out entirely, but failing that, a DamageModeReportDeltaRectanglesFixed would work. (In reply to comment #7) > XFixesFetchRegion lets us find out which rectangles on the screen are included > in a region. What I'm talking about is copying the *contents* of those > rectangles out into a client. So that would be a XGetImage equivalent that takes a region instead of a rectangle, right? Right, that's what would be needed for such apps to "process all of the rectangles in a single operation". *** Bug 15347 has been marked as a duplicate of this bug. *** -- GitLab Migration Automatic Message -- This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity. You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/xorg/xserver/issues/361. |
Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.
damageproto.txt, when describing the DamageSubtract request, states: DamageSubtract [...] Synchronously modifies the regions in the following manner: (A) If repair is None: 1) parts = damage 2) damage = <empty> (B) Otherwise: 1) parts = damage INTERSECT repair 2) damage = damage - parts 3) Generate DamageNotify for remaining damage areas (Letters added for ease of reference.) My issue is with point B.3. In my application[1], I request damage mode DamageReportDeltaRectangles. I accumulate these damage notifications in a GdkRegion in my client, and from time to time a separate socket becomes writeable and I pick a rectangle from my Region, subtract it from the server damage region (to indicate that I would like to re-enable damage notification for that rectangle), and send the image of that rectangle down my socket. This all works nicely, except for B.3: every time I re-enable damage notifications on one rectangle with DamageSubtract, I get a giant flurry of pointless notifications for damage rectangles I have already recorded. Then I handle the next rectangle, and get another flurry... ultimately this makes a simple redraw operation O(n^2), and in real-world cases can cause my program to thrash for several minutes to deliver a single screen update. The only way DeltaRectangles mode makes any sense to use at all is if one has this basic strategy of accumulating damage locally and issuing DamageSubtract when a piece of it is handled. Given such a strategy, re-issuing notifications is utterly pointless. A simple but inefficient workaround is to use mode RawRectangles; for this mode the current X server violates the spec and makes DamageSubtract a complete no-op. I am not entirely certain what the right fix here is: Option 1: Stop re-issuing notifications, as per B.3, for damage level DeltaRectangles. Option 2: Just kill B.3 entirely -- never re-issue damage notifications. Either of these will solve my problem, obviously; the question is which is cleaner, and in particular whether there is any use case where the re-issued damage notifications are valuable. They seem to be in the spec entirely for the benefit of mode NonEmpty, but I can't figure out how one could use them without race conditions. If one is using notification mode NonEmpty, one should be setting repair=None anyway, and avoiding this whole problem... Maybe Keith or Eric, whose names are on the spec, can explain the rationale? Also, looking through all the different uses of DamageSubtract in google codesearch, it appears that they fall into two classes: -- traditional cm's, which use NonEmpty and set repair=None, so they take branch A and B.3 is irrelevant. -- apps that want to keep track of what's on the screen (like xpra, and byzanz, and vino), which use DeltaRectangles and are bitten by this bug. So just removing damage re-notifications entirely seems like a viable course of action. Anyway, what I'd request: -- damage re-issuing be fixed for, at least, DeltaRectangles mode for 7.4 -- the Damage protocol version get bumped to 1.2 so that apps can tell that DeltaRectangles is usable Let me know if there's anything I can do to help this occur. [1] http://partiwm.org/wiki/xpra