Ticket UUID: | 685ac3072790118c07db73c1cb667e8bd37bddf8 | |||
Title: | Aqua: XCopyArea() broken for non-pixmap source (as used by Tktable) | |||
Type: | Bug | Version: | 8.6.10 | |
Submitter: | chrstphrchvz | Created on: | 2020-04-08 08:32:55 | |
Subsystem: | 82. X11 Emulation | Assigned To: | nobody | |
Priority: | 5 Medium | Severity: | Minor | |
Status: | Open | Last Modified: | 2022-09-01 07:58:44 | |
Resolution: | None | Closed By: | nobody | |
Closed on: | ||||
Description: |
Tktable bug report from 2010: https://sourceforge.net/p/tktable/bugs/309/ In Tktable, a cell's text "overflows" when the text is wider or taller than the cell. Under Tk Cocoa, any cells with overflowing text will have their text erroneously appear behind the text of every later cell (i.e. a cell in a later column in the same row, or a cell in a later row) with overflowing text, unless The problem is more clearly seen by overflowing cells with whitespace and/or non-overlapping text:
Examining the drawing code in So this would indicate As I understand it, Probably the only case where Or, have Tk draw everything into an image (which Tk is free to read from), and then setting that image as what is drawn into the backing store, or directly used instead of a backing store. This seems possible (CALayer/layer-backed view using Otherwise, this issue is in some sense unfixable; Is it important for Tk Aqua to support this and similar X11 emulation functions, or is it not worth the effort? Tktable has been dormant for several years. But it seems possible for it to implement a workaround; I will post some ideas to the Tktable bug report. | |||
User Comments: |
chrstphrchvz added on 2022-09-01 07:58:44:
The NSImage drawing approach I discussed here is reborn as the CGBitmapContext/CGImage approach proposed in [3e9e82bcfe4b]; part of it is to get XCopyArea() and XGetImage() working again. chrstphrchvz added on 2021-04-12 04:45:42: CFRelease(bitmapRep) should probably have instead been [bitmapRep release] since bitmapRep is a kind of NSObject *. chrstphrchvz added on 2021-04-11 03:07:45: Without seeing a stack trace for the crash reported on tcl-mac, here's why I think it happens: image create photo -type window relies on XGetImage(), which in Tk Aqua in turn relies on CreateCGImageFromDrawableRect() (tkMacOSXImage.c). This line in CreateCGImageFromDrawableRect(): } else if (TkMacOSXGetNSViewForDrawable(mac_drawable) != nil) { used to be (when it was in TkMacOSXBitmapRepFromDrawableRect() ()): } else if ((view = TkMacOSXDrawableView(mac_drawable)) != NULL) { but in [b3409e6717] became (I believe mistakenly): } else if (TkMacOSXDrawableView(mac_drawable) != NULL) {This currently leaves view to always be nil even when TkMacOSXGetNSViewForDrawable(mac_drawable) != nil. And since any image create photo -type window usage more than likely happens in the normal Tcl event loop and not while in drawRect:, [NSView focusView] == nil. Therefore view == [NSView focusView] and we end up doing [view cacheDisplayInRect:view_rect toBitmapImageRep:bitmapRep] which just sets bitmapRep to NULL, causing CFRelease(bitmapRep) to crash ("illegal hardware instruction"). In 0003-Aqua-Restore-initWithFocusedViewRect-usage.patch I restore the assignment to view: } else if ((view = TkMacOSXGetNSViewForDrawable(mac_drawable)) != nil) { This change alone could be applied right away: it will not resolve the larger issue of not being able to capture windows, but will at least error ("Window … cannot be transformed into a pixmap (possibly obscured?)") rather than crash. chrstphrchvz added on 2021-04-11 01:59:29: The issue of no longer being able to capture a window on Aqua using TkImg has been reported by another user on the tcl-mac mailing list: https://sourceforge.net/p/tcl/mailman/tcl-mac/thread/c3e00748-e0d6-fc08-ce85-6160de94ddb8%40mastersoft.com/#msg37258979 chrstphrchvz added on 2020-11-15 11:26:31: A better solution was found for the Tktable issue in the description (see [b505e5f6a9]), although the issues with Tk Aqua's XCopyArea() and XGetImage() remain. I have attached refreshed patches for reinstating initWithFocusedViewRect:, which apply to the latest core-8-6-branch ([cfb1c35782]). Here is a demo script for testing screenshot of a widget with TkImg: package require Tk package require Img Running this and pressing "Capture this button" should open a new toplevel with the screenshot of the button when it was clicked; the red background surrounding the button should not leak into the screenshot. Someone with both a Retina and a non-Retina display should be able to check that the demo works even after moving the root toplevel between displays. I have only had success with this demo on macOS 10.13, and not on (modified) 10.15, which may indicate that initWithFocusedViewRect: no longer works as of 10.14. But this implementation should be better than the current one, which doesn't work on any macOS version. From Marc's comment on [fcd6717d8b] (2020-08-23 02:40:15): I probably misread the code, but it looked like the Retinaciousness was being stored in a global variable, so it would not be sensitive to a window being moved from one display to another. More importantly, though, what do you see as the issue with simply asking an NSWindow for its backingStoreScaleFactor whenever that is relevant? I believe this is referring to the tkMacOSXNSWindowHasBackingScaleFactor global variable, which only indicates whether the backingScaleFactor of NSWindows can be accessed (which is only possible on macOS 10.7 and later). The backingScaleFactor of an NSWindow is being asked for when needed, using the TkMacOSXNSWindowBackingScaleFactor() wrapper function (although if running macOS 10.6, the function will always return 1.0); the backingScaleFactor is not then cached elsewhere by Tk. chrstphrchvz added on 2020-08-18 02:25:48: In response to my comment from 2020-05-24 23:23:57: only calling initWithFocusedViewRect: while in drawRect: will only work for usage that can wait until inside drawRect: before calling it, which is the case for Tktable. But lockFocus and unlockFocus are still necessary if something needs to retrieve from the backing store while outside of a drawRect: call for the drawable's view, such as capturing a window with TkImg (it cannot wait until or try again when in drawRect:). chrstphrchvz added on 2020-08-18 02:05:51: I have attached patches based on what I currently had in the iwfvr-no-cdir branch. Patches #3 and #6 are the important ones; the rest are comment tweaks and optional refactoring. chrstphrchvz added on 2020-08-15 02:02:34: In case I don't get around to attaching patches soon, please have a look at my work so far on a branch on my GitHub fork: https://github.com/chrstphrchvz/tk/commits/iwfvr-no-cdir One set of bugs I would like to simultaneously address is the instances where the imageBounds argument for TkMacOSXDrawCGImage() is invalid: it is being initialized with (MacDrawable *)srcDraw->size which is NSZeroSize for non-pixmaps. This leads to TkMacOSXDrawCGImage() complaining about "Mismatch of sub CGImage bounds." Although there may not be anything broken in TkMacOSXDrawCGImage(), one suggestion I have would be to simplify it so that it doesn't even accept imageBounds nor srcBounds. I would leave TkMacOSXDrawCGImage() to mainly be a wrapper around CGContextDrawImage(), meaning that any cropping of the source image would need to be done before calling it, and the image will be implicitly rescaled by CGContextDrawImage() if its size does not equal dstBounds.size. Only XPutImage() would need cropping logic added (using CGImageCreateWithImageInRect()); other instances can/already use TkMacOSXBitmapRepFromDrawableRect() to crop the source. To separate out and reuse some refactoring from the NSImage→CALayer drawing experiment, I also propose renaming TkMacOSXBitmapRepFromDrawableRect() to TkMacOSXCreateCGImageFromDrawableRect() and having it return a CGImageRef instead. marc_culler (claiming to be Marc Culler) added on 2020-08-14 15:45:33: A patch would be great! chrstphrchvz added on 2020-08-12 22:07:00: I made a separate installation of macOS 10.13 High Sierra (the last version officially supporting my 2010 MBP), and using initWithFocusedViewRect: on it appears to work without the crashes or glitches I observed on 10.15. I think initWithFocusedViewRect: usage can be reinstated at least for 10.13 and earlier. Maybe it can be safely used on supported Macs running 10.14 and later, if someone else can verify (and if Tk would rather not workaround issues due to running macOS on unsupported Macs). At a minimum, I would still like to remove cacheDisplayInRect: usage, as that is algorithmically inappropriate for Tk, and deliberately allow nil to be returned instead. I should be able to provide patches for this. As I did not change very much to get initWithFocusedViewRect: working, I'm prompted to think I may have incorrectly assumed that this issue is the same one reported for Tktable 10 years ago. There may instead be two issues: a new one introduced as recently as Tk 8.6.9.1 (in response to initWithFocusedViewRect: being deprecated on 10.14 Mojave); and a 10-year-old one whose steps to reproduce and workaround (-drawmode slow) coincidentally apply to the new issue, and which may have actually been addressed by Tk at some point (without anyone following up on the Tktable ticket). All the reports of the new issue that I'm aware of are with Tk 8.9.6.1 or later; and I never confirmed exactly what the old issue looked like—the reporter of the SourceForge ticket never provided a screenshot of the issue, and the Tktable maintainer only described it as "something about the cocoa text drawing is defying the general clipping routines", which isn't quite how I would describe the new issue. chrstphrchvz added on 2020-08-09 18:50:23: I have pushed my work so far on NSImage→CALayer drawing to a branch on GitHub (careful, commits may get rebased and disappear): https://github.com/chrstphrchvz/tk/commits/calayer-drawing Overview of the NSImage→CALayer drawing approach so far: An NSImage is created with the size of the toplevel's TKContentView instance; for convenience a reference to it is stored in the tkLayerImage property of the TKContentView. updateLayer and wantsUpdateLayer are overridden for TKContentView: updateLayer is where tkLayerImage is used to set the contents of the TKContentView's layer; and wantsUpdateLayer returns YES so that updateLayer is called instead of drawRect:. setWantsLayer:YES is called for each TKContentView instance. TkMacOSXSetupDrawingContext() and TkMacOSXRestoreDrawingContext() are modified to call lockFocus and unlockFocus respectively on tkLayerImage. This means that widgets should be able to draw into tkLayerImage immediately rather than only during deferred drawing as with drawRect:. TkMacOSXRestoreDrawingContext() calls addTkDirtyRect: for the bounds of the TKContentView instance so that updateLayer is eventually called. Unlike drawRect:, no widget redrawing routines execute during updateLayer. updateLayer calls clearTkDirtyRect when it finishes. TkMacOSXBitmapRepFromDrawableRect() (renamed to TkMacOSXCreateCGImageFromDrawableRect()) is modified to take advantage of being able to retrieve prior drawing from tkLayerImage as a CGImage, which is then usable by functions including XGetImage() and XCopyArea(). Tweaks were made to other functions including TkMacOSXDrawCGImage(), which may be useful outside of this experimental branch for addressing existing bugs.
Here are Retina display screenshots of overflowing Tktable text without -drawmode slow, and a button widget captured using TkImg and then displayed in a separate toplevel:
A few issues I'm aware of:
chrstphrchvz added on 2020-08-02 19:16:10: I tried switching Tk back to using initWithFocusedViewRect:, and I'm not sure it can work. On my MacBook Pro 2010 running (modified) macOS 10.15.6, calling this method enough times eventually triggers a crash in ERROR_CGBlt_copyBytes_BufferIsNotReadable or ERROR_CGBlt_copyBytes_BufferIsNotBigEnough. I haven't tried seeing what happens on another machine/graphics card, so I can't rule out that these errors are due to using graphics card drivers taken from an older macOS version and modified for macOS 10.15 (which doesn't support this Mac or its graphics card). I am going down the rabbit hole of getting Tk to draw to an NSImage (or possibly some other image type) which then provides a CALayer to use as an NSView's contents, rather than drawing directly into an NSView. Having access to an image should allow doing things either issue-prone or not possible with an opaque backing store. This also involves eliminating scrollRect:by: usage. Besides this Tktable example, I am also trying to use TkImg to get an image of a window, which involves getting the XGetImage() and TkMacOSXBitmapRepFromDrawableRect() functions to finally work for real windows (i.e. not just pixmaps). This experiment is revealing quite a few other smaller bugs along the way. For now the goal of this experiment is correctness rather than performance; so far this approach runs significantly slower. Some of the functionality may only exist in macOS versions more recent than 10.6. chrstphrchvz added on 2020-05-25 14:04:14:
I still have yet to try anything, but there are reports of marc_culler (claiming to be Marc Culler) added on 2020-05-25 00:05:24: That sounds like an excellent idea, if it works. There might be some unanticipated problem, but it is definitely worth testing. Apple's minimalist documentation does not give one much to go on, so trial and error is really the only option as far as I can tell. The nature of the backing store changed with the Mojave release, according to Apple. But is not clear to me what the implications of that may be. chrstphrchvz added on 2020-05-24 23:23:57:
If Tk does go back to using marc_culler (claiming to be Marc Culler) added on 2020-05-24 23:05:29: I agree that switching back to initWithFocusedViewRect: would be a good idea, assuming that it actually works. At the time it seemed like a good goal to avoid all deprecated APIs, especially when a replacement is provided. But the replacement did not work. And now we are using other deprecated apis ([view scrollRect:by:]) which do not have any replacement. In the case of [view scrollRect:by:] no deprecation warning is issued when building with target system 10.9, which is a common choice these days. So it would seem that Apple is likely to keep that api available at least until they decide to completely abandon 10.9. Probably the situation with initWithFocusedViewRect is similar. chrstphrchvz added on 2020-05-24 20:21:08:
Apparently, there has been a way to read from a Mac window's backing store without calling I'm under the impression that Apple has been deprecating APIs not to signal that they intend to remove them (unless they explicitly state so), but rather because Apple found too many programs out in the wild which they believed were using APIs incorrectly or should instead use some strongly-preferred (and possibly more performant) alternative—in, say, 99.9% of programs. But the strongly-suggested alternative may neither be drop-in replacements for deprecated APIs, nor even provide the functionality that the remaining 0.1% of programs truly require. I think Tk has good reason to keep using the Besides chrstphrchvz added on 2020-04-13 15:21:25:
…using
I have opened [e2e9ce70b2] regarding this. chrstphrchvz added on 2020-04-11 08:36:13: I used the basic.tcl demo for the screenshots. I either shrink rows/columns or add text in a cell until the text overflows. The Tktable man page says The marc_culler (claiming to be Marc Culler) added on 2020-04-11 00:27:50: The first question that occurs to me is whether -drawmode slow is *actually* slow, or is that just a name that was chosen for the option. If it is as fast, or only a tolerable amount slower, then the obvious workaround would be to always use that mode when running in Aqua. If I were to look at this it would be helpful to have an easy (for me) way to produce an example of the problem. Could you perhaps post the script that was used to generate the images in this ticket? |
Attachments:
- 0003-Aqua-Restore-initWithFocusedViewRect-usage.patch [download] added by chrstphrchvz on 2021-01-17 10:26:43. [details]
- 0005-Aqua-X11-emulation-support-Retina-window-source.patch [download] added by chrstphrchvz on 2020-11-15 10:47:41. [details]
- 0004-Simplify-TkMacOSXDrawCGImage.patch [download] added by chrstphrchvz on 2020-11-15 10:47:34. [details]
- 0002-Aqua-add-wrapper-for-NSWindow-backingScaleFactor.patch [download] added by chrstphrchvz on 2020-11-15 10:47:10. [details]
- 0001-Aqua-X11-emulation-Adjust-some-comments.patch [download] added by chrstphrchvz on 2020-11-15 10:47:01. [details]