Skip to content

Conversation

@chrstphrchvz
Copy link

shapePhoto() currently works by obtaining a region from TkPhotoGetValidRegion() and passing it to Shape_CombineRegion() (and in turn XShapeCombineRegion() on X11). However, TkPhotoGetValidRegion() is not a public Tk C API. And as discussed at https://core.tcl-lang.org/tk/info/919066 it takes a very long time to compute the region for some images, and I am not aware of an instance in core Tk or an extension where using this region is strictly necessary. Tk could draw photo image instances using a clip mask rather than a region (as attempted in chrstphrchvz/tk#4). I propose that shapePhoto() follow a similar approach: use the public API Tk_PhotoGetImage() to access alpha channel data and create a bitmap (pixmap with depth of 1) for use a clip mask to be passed to Shape_CombineBitmap() (and in turn XShapeCombineMask() on X11).

Note that this would leave Shape_CombineRegion() unused within the shape extension.

If Tk 8.3 or later can be required, then this approach allows photo shape source support to be enabled unconditionally. That should also make it okay for demos to use a photo shape source. As an example, I propose revising the dragger.tcl demo: instead of using doc-mask.xbm as a bitmap shape source, make doc-img.gif have a transparent background and use it as a photo shape source; the demo should look the same as before.

I have tested this using recent Tk core-8-6-branch on XQuartz (macOS), after applying demo and build fixes/workarounds (some from #7, some not reported). Some build fixes are currently included in this pull request, but can be moved to another pull request if desirable.

@dkfellows
Copy link
Owner

Requiring post-8.3 is trivial these days. Anyone using a version of Tk that doesn't have an alpha channel in photo images is also quite able to use the old versions of this code.

shape.c:404:47: error: cannot take the address of an rvalue of type 'void *'
  404 |     imageName = Tcl_GetStringFromObj(objv[0], &NULL);
      |                                               ^~~~~
Remove unneeded includes, macros, and configure checks
@chrstphrchvz
Copy link
Author

Rebased and revised to have one less potential conflict with ff6738d from #9.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants