A few minor code style issues were also fixed.
Note that there's only one procedure parameter (Preset name),
but it's not retained between runs.
I want to ask about this before making a full conversion to
GimpProcedureDialog widgets.
This name was really irking me because it's not a button (anymore? Maybe it used
to be just a button). Depending on the specific widget, it will have several
sub-widgets, including a label. And it can theoretically even be something else
than a button.
So let's just rename these widgets with the more generic "chooser" name.
Similar to the latest commits for GimpBrush:
- gimp_pattern_get_buffer() returns a GeglBuffer and allow getting a scaled
version of the pattern.
- Old gimp_pattern_get_pixels() is made private.
- Moved GimpPattern into its own file and store the buffer to avoid re-querying
it through PDB continuously.
No as for the widget to select a pattern:
- Preview frame ensured to be square.
- Default size increased.
- Drawing code using the new gimp_pattern_get_buffer().
- Cleaned up code.
So what I realized was that the core was sending contents without transparency.
Actually the mask was our transparency channel here. Since in most use cases,
what you want to do when you request a brush buffer is to be able to draw it
somewhere, having a buffer already with alpha is much better, even more because
by default, it looks like background color is black which is possibly not what
you expect usually from a brush preview.
If someone wants absolutely no-alpha, it's easy to get rid of the channel. It's
simply better that the default behavior is the most expected use case.
- Increase the default size to 40x40 and multiply it by the current window scale
factor to have decent preview size.
- Make the brush preview always square with a GtkAspectFrame: even though
brushes are not necessarily square, this is a much more obvious size rather
than letting GTK choose a random allocation size which ends up very weird
looking.
- Scale down the brush to the biggest possible dimensions which fit the square
preview area (if the brush native size is already smaller, I don't scale up
though) while keeping aspect ratio: previous implementation was really weird,
as we were only seeing a tiny corner of much brushes as we weren't scaling
them down. Obviously I use new gimp_brush_get_buffer|mask() functions for
this as it supports scaling.
- Implement drawing color brushes too: the previous implementation was only
drawing the brush mask, which was absolutely not what would be expected for
such brushes.
- Add a white background behind color brushes with transparency.
- Simplify and clean up the code.
One of the consequences of this new implementation is obviously that it's
mandatory to call gegl_init() when using this widget.
… and gimp_brush_get_mask().
gimp_brush_get_pixels() was a bit crappy, returning raw data with only
dimensions and bpp to go with (no color model/space, no bit depth…). So the
assumption is that we work with 8-bit per channel data, possibly with alpha
depending of number of channels as deduced from bpp, and very likely in sRGB
color space. It might be globally ok with many of the brush formats (and
historical brushes) but won't fare well as we improve brush capabilities.
- gimp_brush_get_pixels() is in fact made private.
- The 2 new functions are using this old PDB call _gimp_brush_get_pixels() to
construct buffers. This has some limitations, in particular that it returns
only 8-bit per channel sRGB data, but at least the signature won't change when
we will improve things in the future (so if some day, we pass fancy brushes in
high-bit depth, the method will stay the same).
- This new implementation also allows scaling down the brush (keeping aspect
ratio) which is useful when you need to fit a brush preview into a drawing
widget.
- Current implementation stores the buffers at native size in the libgimp's
GimpBrush object, hence save re-querying the core every time you need an
update. This can be improved as current implementation also means that you
don't get updates if the brush changed. This should handle most common use
cases for now, though.
- Also with this change, I move GimpBrush class implementation into its own
dedicated file.
- Move the property widget functions for GimpResource properties into a new
libgimp/gimppropwidgets.[ch] file. This mirrors the files
libgimpwidgets/gimppropwidgets.[ch] which are for more generic property types.
- Rename the functions gimp_prop_chooser_*_new() to gimp_prop_*_chooser_new().
- gimp_prop_chooser_factory() doesn't need to be public.
- Add a label to GimpResourceSelectButton, make so that the
gimp_prop_chooser_*_new() functions set the property nick to this label and
add this label to the size group in GimpProcedureDialog.
… from the plug-in normal runtime crossed streams!
I also add a huge comment in-code, because this was annoying enough to
understand and debug that I don't want someone to remove the idle without proper
consideration and testing in the future, thinking it's useless.
- Removing useless or redundant code.
- Simplifying various logics.
- Using GimpResource directly in temporary PDB procedures, not resource names.
- Better cleanup of the core resource chooser when the plug-in dialog quits (we
need it to ask core to close also any visible resource chooser dialog).
- Replace the "Close" button by more common OK/Cancel. In particular, the
GimpPdbDialog now properly keeps track of the initial object and when hitting
"Cancel" (or Escape key), this initial object is set back.
- Clean up some of the comments, especially when the code is self explanatory.
There is still much more to clean and improve, but it's a first welcome step.
Found by the definitely useful libgimp warnings:
> gimp_plug_in_destroy_proxies: ERROR: GimpPattern proxy with ID 13 was refed by plug-in, it MUST NOT do that!
One instance of `(GIMP_VALUES_GET_INT (args, 0)` was missed during the initial conversion.
This caused a compiler error. The value is now retrieved from the GimpProcedureConfig
object.
Brush, font, gradient, palette and pattern choices are currently chosen through
a dialog created by the core, which then returns the user choice to the calling
plug-in. This has the unfortunate consequence of having a pile of likely at
least 3 windows (main GIMP window by core process, plug-in window by plug-in
process, then the choice popup by the core process) shared in 2 processes, which
often end up under each other and that's messy. Even more as the choice popup is
kinda expected to be like a sub-part of the plug-in dialog.
So anyway, now the plug-in can send its window handle to the core so that the
resource choice dialog ends up always above the plug-in dialog.
Of course, it will always work only on platforms where we have working
inter-process transient support.
Instead of passing a guint32, pass the proper type, since our the HANDLE type
can be 64-bit on Windows (according to links I found).
I was hoping it might be the reason for the breakage under Windows, though I
also found Microsoft documentation saying that the 64-bit handle can be safely
truncated: https://learn.microsoft.com/en-us/windows/win32/winprog64/interprocess-communication?redirectedfrom=MSDN
Nevertheless I'd appreciate testing again from NikcDC or anyone else, as I
reactivated setting transient between processes on Windows.
Note that I also pass the proper types on X11 now (Window), even though guint32
worked fine. Better be thorough.
Having windows ID as guint32 is a mistake. Different systems have
different protocols. In Wayland in particular, Windows handles are
exchanged as strings. What this commit does is the following:
In core:
- get_window_id() virtual function in core GimpProgress is changed to
return a GBytes, as a generic "data" to represent a window differently
on different systems.
- All implementations of get_window_id() in various classes implementing
this interface are updated accordingly:
* GimpSubProgress
* GimpDisplay returns the handle of its shell.
* GimpDisplayShell now creates its window handle at construction with
libgimpwidget's gimp_widget_set_native_handle() and simply return
this handle every time it's requested.
* GimpFileDialog also creates its window handle at construction with
gimp_widget_set_native_handle().
- gimp_window_set_transient_for() in core is changed to take a
GimpProgress as argument (instead of a guint32 ID), requests and
process the ID itself, according to the running platform. In
particular, the following were improved:
* Unlike old code, it will work even if the window is not visible yet.
In such a case, the function simply adds a signal handler to set
transient at mapping. It makes it easier to use it at construction
in a reliable way.
* It now works for Wayland too, additionally to X11.
- GimpPdbProgress now exchanges a GBytes too with the command
GIMP_PROGRESS_COMMAND_GET_WINDOW.
- display_get_window_id() in gimp-gui.h also returns a GBytes now.
PDB/libgimp:
- gimp_display_get_window_handle() and gimp_progress_get_window_handle()
now return a GBytes to represent a window handle in an opaque way
(depending on the running platform).
In libgimp:
- GimpProgress's get_window() virtual function changed to return a
GBytes and renamed get_window_handle().
- In particular GimpProgressBar is the only implementation of
get_window_handle(). It creates its handle at object construction with
libgimpwidget's gimp_widget_set_native_handle() and the virtual
method's implementation simply returns the GBytes.
In libgimpUi:
- gimp_ui_get_display_window() and gimp_ui_get_progress_window() were
removed. We should not assume anymore that it is possible to create a
GdkWindow to be used. For instance this is not possible with Wayland
which has its own way to set a window transient with a string handle.
- gimp_window_set_transient_for_display() and
gimp_window_set_transient() now use an internal implementation similar
to core gimp_window_set_transient_for(), with the same improvements
(works even at construction when the window is not visible yet + works
for Wayland too).
In libgimpwidgets:
- New gimp_widget_set_native_handle() is a helper function used both in
core and libgimp* libraries for widgets which we want to be usable as
possible parents. It takes care of getting the relevant window handle
(depending on the running platform) and stores it in a given pointer,
either immediately or after a callback once the widget is mapped. So
it can be used at construction. Also it sets a handle for X11 or
Wayland.
In plug-ins:
- Screenshot uses the new gimp_progress_get_window_handle() directly now
in its X11 code path and creates out of it a GdkWindows itself with
gdk_x11_window_foreign_new_for_display().
Our inter-process transient implementation only worked for X11, and with
this commit, it works for Wayland too.
There is code for Windows but it is currently disabled as it apparently
hangs (there is a comment in-code which links to this old report:
https://bugzilla.gnome.org/show_bug.cgi?id=359538). NikcDC tested
yesterday with re-enabling the code and said they experienced a freeze.
;-(
Finally there is no infrastructure yet to make this work on macOS and
apparently there is no implementation of window handle in GDK for macOS
that I could find. I'm not sure if macOS doesn't have this concept of
setting transient on another processus's window or GDK is simply lacking
the implementation.
...and to GimpProcedureDialog.
selection-to-path-dialog.c was also removed as the code can now be contained
within selection-to-path.c thanks to the new API.
… is set.
The order for thumbnail creation in gimp_imagefile_create_thumbnail() is now:
1. If there is a GimpThumbnailProcedure, it is run first.
2. Otherwise we check if a thumbnail is in the metadata.
3. As last resort, we just load the full image.
Part of the fix was to copy gimp_image_metadata_load_thumbnail() into the core
code. I have been wondering if we could not drop the same function from libgimp
and remove the GimpThumbnailProcedure frome file-jpeg, since it just uses the
metadata thumbnail and it is the only plug-in using this code.
Also it is much faster to run this in core and it's generic function which makes
thumbnail loading from Exif data working for every format supported by Exiv2.
On the other hand, the file-jpeg thumbnail procedure also gathers a few more
useful information, such as the color model (in a reliably manner, since based
on JPEG header, unlike from metadata which may be wrong).
The various information (width, height, image type and number of layers) are
those of the full image, not of the thumbnail. Make it clear in the docs of
GimpRunThumbnailFunc.
Additionally:
- file-xmc was returning the proper information but variables were wrongly
named, which was confusing.
- Fix file-ico thumbnail proc which was returning the thumbnail width/height.
- In file-darktable, initialize width/height to 0 so that we just don't show any
size when we don't get the information. It's better not to show anything than
completely wrong information (the thumbnail target size).
We were choosing the bigger icon in the ICNS list. Instead let's choose the
bigger icon within the target size bounding box. In case there is no smaller (or
equal size) icon, we falls back to the smallest bigger icon.
Also make sure we set width and height to the full image size, as this is used
as information on the full image (not the thumbnail).
SVG is a vector format which is easy to render exactly within the size×size
bounding box. Let's make sure we do so.
This makes for much sharper SVG thumbnails (and also possibly faster thumbnail
render).
… than a GimpValueArray.
Similar to other GimpProcedure, move to using a config object. A difference is
that thumbnail procedures are always run non-interactively.
Also fixing WMF load thumbnail procedure: the dimension computation was wrong
when the image was wider than tall.
Also the palette argument is now a proper GFile argument (not a string).
There is also a palette argument for the export procedure, but it's currently
unused. A palette storing function will need to be implemented.
Some bit of additional logic cleanup was done.
Again, I am losing a tiny bit of UI, in particular the ratio fields, but also
the update of the size label (this was kinda broken anyway, as it updated only
when updating some fields, not others).
Also moving the default resolution to 300 PPI.
Last but not least, I transformed the "paths" int argument to a GimpChoice
argument.
The new dialog is not fully on-par with the old one. We lost the X and Y ratio
fields as well as the ability to constrain dimensions to each other. This should
be improved when we'll get proper automatically generated widgets for dimension
arguments.
… a GimpProcedureConfig for arguments.
This also factorizes the code to load metadata. By default, a GimpLoadProcedure
will try and load metadata from a file (if Exiv2 knows the format). The run()
function will be allowed to edit the GimpMetadata object but also the load flags
before it is actually attached to the image, allowing plug-ins to have custom
metadata handling code when needed.
This is used in the generated GUIs for GimpChoice arguments, but also for
validation of property setting.
New functions:
* gimp_choice_set_sensitive()
* gimp_string_combo_box_set_sensitivity()
This is just a method to simplify transforming a GimpChoice argument into an
enum value, which is easier to deal with, in C. It also allows to benefit from
switch() warnings or the like to make sure no cases are missing.
Developers won't have to maintain manually a list of the possible values in the
help string. It can now be generated from the GimpChoice and will be therefore
ensured to always be up-to-date, and nicely formatted.
I also add some pango markup to the type helper texts to differentiate it from
the main argument docs.
These will replace the int arguments used in place of enums. The problem of int
arguments used as list of choices is that it makes calling PDB functions very
opaque. This is especially bad when a list is long, so you constantly have to
refer to the documentation to understand what a series of numbers mean in
argument lists.
And the second issue is that plug-in developers have to manually maintain a list
of values both in the GUI and in the documentation string. This help text may
get out-of-sync, may end up with missing values or whatnot. Also if it is used
as tooltips, it makes for very weird tooltips in the graphical interface, with
an overlong technical list of int-values mapping which should ideally only be
made visible in the PDB procedure browser listing.
- GimpLabelStringWidget widget makes any widget with a "value" string property
into a GimpLabeled.
- Add a "value" string property to GimpStringComboBox (which makes it usable by
GimpLabelStringWidget).
This will be used for creating limited lists of strings as argument types for
procedures.
Ideally enums are the best type for this, but it can only be used for generic
libgimp* enum types, not custom enums created only for a given plug-in. For
this, we currently just demote the args to ints which lose any semantic. A
limited list of string will give back some semantic and some better validation,
even though it's a tiny bit more annoying to work with strings than int types
(at least in C).
Alx Sa