… gimp_label_spin_set_digits() and deleted gimp_prop_opacity_entry_new()
- The "digits" argument for the number of decimal places in
gimp_prop_scale_entry_new() is now mostly useless since GimpLabelSpin
(hence GimpScaleEntry too) got a nice estimation algorithm of sensible
values.
- Add gimp_label_spin_set_digits() function to manually set the digits
property when we don't like the estimated value.
- Also add a new "factor" argument to gimp_prop_scale_entry_new(). Its
role is to allow a GimpScaleEntry showing a factored range, typically
a [0, 100] range for some types of [0, 1] properties.
- Remove gimp_prop_opacity_entry_new() which was basically a
special-case of gimp_prop_scale_entry_new() and which can now be
easily reproduced by simply set factor=100.0.
- Update all usage of gimp_prop_scale_entry_new() in app/ and plug-ins/
with updated arguments. It is interesting to note that all existing
usage were either integers (digits=1) or when double, the estimated
decimal places are the same as the ones which were manually set (hence
showing the generic estimation is not too bad). So the new function
gimp_label_spin_set_digits() was not even needed once in current code.
A -quick done- first step towards the addition of a smart selection tool.
Require the gegl:paint-select workshop operation.
Still LOT of work to do (wip):
- fluctuations removal (GEGL side)
- multilevels pyramid approach + banded graphcut for instant result on large
image (GEGL ? GIMP ?)
- Gaussian Mixtures for color models (GEGL side)
- drawable offsets (GIMP side)
- undo / redo (GIMP side)
- scribbles edition mode (GIMP side)
- dedicated icons
- ...
… in Image properties window.
Basically using non-translatable "%s (%d %s)" and filling it with
"Indexed color" base type name and translatable _("colors") is very bad
practice (localization-wise):
- First it assumes everyone uses round brackets.
- It also assumes the order of words (number before word it determines
but even the image type before detail brackets).
- And finally it doesn't handle plurals. Of course, we could say that
the 1 color case is a very edge impractical case, but plural is not
only about 1 vs other numbers. Some languages have more cases, and
using ngettext allows translators to handle these.
First of all, "CJK Unified Ideographs" block should not be the highest
priority to determine showing an ideograph. Indeed most fonts for a
Korean and Japanese audience would also contain at least the main
ideographs. So instead, look first for Korean alphabet (Hangul) and
Japanese syllabaries to determine if it's a Korean or Japanese-targetted
font. Only then Chinese.
Also check Korean before Japanese because most of the Korean fonts I saw
actually also include Japanese syllabaries (but not the other way
around).
This way, it will be much easier for CJK graphists to skim through the
font list and detect fonts made for the needed language in a glance.
Also modifying the Korean display text. KIYEOK and SSANGKIYEOK were
obviously chosen because they were the first in the block. But they are
very bad choice. We hesitated with 가 at first, as it is considered the
first in the syllabary form (가나다라 is kind of similar to our ABCD).
But it wouldn't show a form with a second consonant (below) which is a
good stylistic indication. So we hesitated between 한 (han) and 글
(geul, which also means text so it's a nice sample), and finally went
with 한 because of the circle shape in ㅎ (hieut) but also its small
"hat" which has many stylistic variants. So it's quite a good hint of
stylistic choices made by a font designer from just the sample box.
Moreover I switched the block check from "Hangul Jamo" to "Hangul
Syllables" block. "Hangul Jamo" are positional forms of the letters to
dynamically compose syllables (in particular legacy syllables not in use
anymore). Though a feature-full Korean font set would design these, it
is less important than "Hangul Syllables" (pre-composed syllables
design) or "Hangul Compatibility Jamo" (basically the same letters as
"Hangul Jamo" but for standalone usage). Also I actually saw some fonts
made for Korean without "Hangul Syllables" support.
Finally I also added a test for Japanese. I check the Hiragana block
which is most likely the most basic which has to be in any
Japanese-targetted font and use 'あ' (a) as sample text, which is the
first Hiragana syllable and here definitely a good sample text in my
opinion.
I believe that this can still be improved though. Checking only a single
block to determine the probable target language is not necessarily
enough. For instance very complete fonts for Chinese may also design
Korean and Japanese characters, but will also have most CJK blocks and
more ideographs (whereas Japanese/Korean will likely have less). Yet
let's say this is good for now, at least better than before!
pango_fc_font_lock_face() is deprecated since Pango 1.44.
This may hopefully also fix#5922 as I completely changed the code where
the CRITICAL happened. Yet I left g_return_val_if_fail() to check if the
Harfbuzz font and FreeType face variables are not NULL (because looking
at the code, it looks like these functions returning NULL actually means
there is a bug in the code).
Nevertheless if it turned out that there are non-bug cases where these
could return NULL (for instance a broken font file?), then probably we
should not use g_return_val_if_fail(), but instead address the data
issue in a nicer way.
Bumping harfbuzz dependency to 1.0.5 for hb_ft_font_set_funcs(). Without
configuring the Harfbuzz font with it, hb_ft_font_get_face() always
returns NULL.
Note that it looks like hb_ft_font_lock_face() would actually be better,
but this requires harfbuzz 2.6.5 from last April which is quite recent.
So let's just use the get_face() variant for now.
The color space label may be a bit long (depends on profile title which
may just be anything and we don't control it), so I allow it to wrap.
The file path on the other hand would not work well with wrapping. It
already has ellipsis in center, but GTK always gives the max size it can
as a default. So if the file is even just slightly deep in the file
tree, we end up with extra-wide Image Properties dialog.
My trick is to give a sensible max size at dialog creation (25
characters max) but to disable this max size as soon as the window gets
realized, hence allowing the label to actually grow up to the contents
actual max size, were one to manually resize the window.
"RGB color" is not a color space, only the model. To get full color
space information, we want to display the model and associated profile
name.
Of course, the "Image Properties" dialog also has a tab displaying
details about the color profile. Still it's better if the general info
displays not too wrong label contents.
Note: when the used color profile has no name, it will show "(unnamed
profile)" which is still more informative than just the color model.
- Set the software as `initialized` later, and in particular after all
recovered images (from crash) then all command line images were
opened. The reason is that the DBus calls have necessarily been made
after GIMP was started (typically could be images double-clicked
through GUI). We don't want them to appear before the images given in
command line (or worse, some before and some after).
- Process DBus service's data queue as a FIFO. The image requested first
will be loaded first.
- When a DBus call happens while GIMP is not initialized or restored,
switch to a timeout handler. The problem with idle handlers is that
they would be attempted too often (probably even more during startup
when no user event happens). This is good for actions we want to
happen reasonably quickly (like would be normally DBus calls), but not
when we are unsure of program availability schedule (i.e. at startup).
Here not only the handler would run a lot uselessly but it would
likely even slow the startup down by doing so. So while GIMP is not
initialized, switch to half-a-second timeout handler, then only switch
back to idle handler when we are properly initialized and GIMP is
ready to answer calls in a timely manner.
… DBus calls.
In particular, Aryeom would start GIMP and directly double click some
image to be loaded in GIMP in the very short while when splash is
visible. Previous code would wait for the `restored` flag to be TRUE.
This was nearly it as we can actually start loading images as soon as
the 'restore' signal has passed. Yet the flag is set in the main
handler, but we actually also need the <Image> UI manager to exist,
which is created in gui_restore_after_callback() (so also a 'restore'
handler, yet after the main signal handler, i.e. after `restored` is set
to TRUE). Without this, gui_display_create() would fail with a CRITICAL,
hence file_open_with_proc_and_display() as well.
I could have tried to set the `restored` flag later, maybe with some
clever signal handling trick (and handle both the GUI and non-GUI cases,
i.e. I cannot set the flag inside gui_restore_after_callback() as it
would break the non-GUI cases). Instead I go for a simpler logics with a
new `initialized` flag which is only meant to be set once, once
everything has been loaded, i.e. once you can consider GIMP to be fully
running hence ready to process any common runtime command.
This is a continuation of #5888 as I realized that most layer modes were
fixed with my commit b3fc24268a (and follow-up f40dc40cbc) but at least
2 were still crashing GIMP: "Luma Lighten/Darken only" modes.
There were 2 bugs here:
* The first bug was that when gimp_operation_layer_mode_real_process()
ran, gimp_operation_layer_mode_prepare() had not been run yet.
prepare() is called before the process() of GeglOperation, but it
would seem the process() of GimpOperationLayerMode on the other end
happens before GeglOperation's prepare() is run. I am absolutely
unsure if this is expected or not and have a hard time figuring out
all the details of the C/C++ cohabitation.
As a solution, I am moving out the fish caching (the needed part
inside gimp_operation_layer_mode_real_process()) in its own function
so that I can easily call it separately before inspecting the fishes.
* The second issue was that some blend functions needed more than a
GeglOperation alone. E.g. blend_function() for luma lighten
gimp_operation_layer_mode_blend_luma_lighten_only() would call
gegl_operation_get_source_space() which requires the node to exist.
Similarly for the Luma darken only mode. So I keep both the node and
operation around, and when finalizing, I free the node (which in turn
frees the operation).
Ell > if you are reading our commits, I would really appreciate your
review (or fixes) of my code here! :)
My previous commit broke the autotools build. Apparently when using
g_object_unref(), some C++ symbol leaked into libapppaint.a archive
library, hence the main binaries (e.g. gimp-2.99) could not be linked
without adding -lstdc++ flag:
> /usr/bin/ld: paint/libapppaint.a(gimppaintcore-loops.o): undefined reference to symbol '__gxx_personality_v0@@CXXABI_1.3'
> /usr/bin/ld: /usr/lib/x86_64-linux-gnu/libstdc++.so.6: error adding symbols: DSO missing from command line
Not exactly sure why using this GLib function in particular caused this,
but let's just try another approach in order not to link the main binary
with C++ standard lib.
Instead let's manage all GeglOperation allocation in gimp-layer-modes.c
by adding a gimp_layer_modes_exit() function and some static array for
storing operation object of each layer mode.
The GimpOperationLayerMode variable member in DoLayerBlend was not
properly constructed. C++ class constructor can be called by creating
object variables, but with GObject, we have to do it with pointers.
Otherwise here we were only allocating the memory for the struct, but
not actually calling any initialization functions.
Also it would seem that the struct was not initialized at zero, as the
space_fish variable was not NULL when it should (i.e. even with same
composite and blend space), hence composite_to_blend_fish was not NULL
and since the operation was not a valid GeglOperation when entering
gimp_operation_layer_mode_real_process(), we crashed.
Not sure how it went unseen for so long!
So instead let's make the layer_mode class member into a pointer. As
such, I have to properly allocate and free it. This is also why I am
adding a copy constructor which will ref the pointer (otherwise we unref
more than we ref as the default copy constructor would just copy the
pointer).
By just redirecting to parent signal handler, it was not properly
grabbing focus. In particular for a text entry with number input, we
want the text to be fully selected (because when we go for inputting
numbers, it is more often than not because we want to enter free numbers
very different from existing value).
So let's grab the entry focus, hence fully select current number
contents.
Renaming the temporary function gimp_scale_entry_new2() into
gimp_scale_entry_new() now that the original code is entirely gone. This
is now a fully-fledged widget with a nice and proper introspectable API.
Instead of setting always manually the step and page increments when
creating a GimpScaleEntry, let's just generate some common cases
automatically. Indeed the increments are rarely something you want to
care about. The algorithm used is:
- For a range under 1.0, use a hundredth and a tenth (typically a [0,
1.0] range will step-increment of 0.01 and page-increment of 0.1).
- For small ranges (under 40), step-increment by 1, page-increment by 2.
- For bigger ranges, step-increment by 1, page-increment by 10.
For use cases when you absolutely want specific increment values, I add
the gimp_scale_entry_set_increments() function. It is much better to
have a small and understandable constructor call followed by
configuration calls (only when needed) rather than a constructor with a
crazy amount of parameters. Hence gimp_scale_entry_new() went from 17
arguments (absolutely unreadable calls) to now 5.
* Add a gimp_scale_entry_get_value() because if we don't do a property
widget, getting the value of the widget easily is a main point.
* Move gimp_scale_entry_(set|get)_logarithmic() to the new class API.
* Internally edit the GtkSpinButton width depending on min/max values,
place digits, and possible value sign.
* Emit a "value-changed" signal (similarly to other widgets with a
value), for cases when just binding the "value" property is not
enough.
* Finally use the new API in palette-import-dialog.
It is more accurate to say it returns a list of parasite names rather
than a list of parasites (as we could take it as meaning a list of
GimpParasite). Of course, we would soon see the actual element contents
(if not for the introspection metadata (element-type gchar*)), but
better being accurate in textual docs too.
… instead of gimp_pdb_is_canonical_procedure().
The later would set an error saying "Procedure name '%s' is not a
canonical identifier". Yet the data label is not a procedure name. It is
a random name. I'm not sure why we need it to be canonical too, but why
not. In any case, let's use the right function.
Some people had been complaining that they couldn't find some actions in
some case, which was only because they were in states where the actions
were non-sensitive. So it was "normal" (i.e. not a bug), yet I can see
how it can be disturbing especially when we don't realize that an action
is meant to be inactive in some given case.
Of course the option to show all actions already existed in the
Preferences. But as most options in Preferences, this is hardly
discoverable and many people only use default settings. Moreover showing
hidden action made the action search cluttered with non-sensitive
actions in the middle of sensitive ones.
This change gets rid of the "Show unavailable actions" settings and
always show all matching actions. In order not to clutter the list with
useless results, I simply updated the display logics to always show
non-sensitive action after sensitive ones. Note that even non-sensitive
actions will still be ordered in a better-match-on-top logics, yet they
will be after sensitive actions. So the top results will be the best
matches among sensitive actions (action in history), followed by various
levels of matches (actions with matching labels, tooltips, different
order matches, etc.); then they will be followed by best matches among
non-sensitive actions, followed by the same levels of matches as
sensitive ones.
This way, we still keep a very relevant result and there is no need to
have a settings for this.
There were still a few references to functions which have been removed
from GIMP 3 (because they were deprecated in previous versions), which I
found as I was doing an inventory of removed functions.
This is not a fix, only an extra-ugly workaround so that at the very
least we don't end up with a splash screen taking the whole display on
Wayland.
Basically by setting 1/3 as the max splash size, a Wayland desktop with
no scale ratio will have a splash taking a third of the screen while it
would take 2/3 of the screen with a scale ratio of ×2 (of course, it
will still be very broken with a scale ratio of ×3 but are there
displays needing such high scaling?). The real fix will be when GTK/GDK
fix their API so that it returns what the docs says it should (i.e. a
size in "application pixels" not "device pixels"), as it does on X11,
Windows, macOS… Then we won't create random max size and we will be able
to properly control our splash size.
Note that this neither fixes nor works around the position issue on
Wayland (in my case, the splash was just always on top-left of the
display).
These small glitches have bothered me for a while now, so I finally
fixed these before the dev release!
Basically there were 2 fixes:
1. use the ink extents to compute any drawn area as this is what will be
actually drawn.
2. Not only expose the drawn area of the new text, but also the one of
the previous text in order to be sure all text pixels are correctly
reset (in case the new text is smaller than previous one). I.e. we
must expose the smallest rectangle containing both previous and new
area of text.
As AppStream docs says, <description> "tag should be translated
by-paragraph" in upstream metadata, which is what we have always done
(i.e. <_p> tags which becomes <p xml:lang="xy">).
Unfortunately as_app_get_description() is optimized to work for
Collection Metadata where the 'tag is translated "as a whole"' (again
cf. specs) for faster parsing. So we were ending up with a text mixing
the original and all localized texts.
I have opened a bug report to appstream-glib:
https://github.com/hughsie/appstream-glib/issues/381
While waiting for this to be fixed (i.e. when the function will handle
both cases accordingly to the metadata source), this code makes my own
locale extraction (defaulting to original text which is assumed to be
the previous same level tag with no xml:lang if no tag with the exact
lang attribute was found).
Stable versions (i.e. minor version number even, e.g. 2.10.22) will only
look for higher stable releases. But for unstable versions, we will want
to look up the development releases too. So for instance GIMP 2.99.2
will warn if the development version 2.99.4 has been released, but also
if the stable version 3.0.0 has been released (whatever is the highest,
which is the stable version in this example).
I had recently created gimp_item_is_ancestor() but realize it duplicates
gimp_viewable_is_ancestor() (which works on GimpItem since it's a parent
class). No need for duplicate code.
Thanks to Wormnest for pushing me to look further. Since gimp-file-save
is actually redirecting the call to another procedure (save proc for the
specific format) which might have more arguments, including string
arguments. When it finds any, it sets it to an empty string "" (which I
guess is ok as "default value when we don't know what to put there").
The previous code would not hurt. Starting at the fourth argument
(GFile), it would just do nothing, then continue with the firth and
further. Still, starting directly at the fifth arg is the proper code
for this.
The About dialog refreshes the release information relatively to
currently running version before being displayed. No check of the remote
JSON file is done, it only verifies whether any stored released
version/revision is not same (or even lower) than the actually running
version. Indeed existence of a stored release means that a newer version
exists at check time only. On later run, this stored information may
have become deprecated.
First the deserialize data of extensionrc was wrong. I need to edit the
pointer to the GList (and dereference it when I need).
Also when inserting an extension into the `running_extensions` hash
table, I could not reuse the same string as in the `processed` list
(because this string is going to be freed at end of initialization).
Just always use the GimpObject name of the extension, since it will stay
alive for as long as the object is alive.
g_list_find_custom() uses a GCompareFunc which has a return value
similar to strcmp(), i.e. with 0 for equality (and not a boolean, which
is basically the opposite).
When loading a theme on Windows we always get an error like:
themes_theme_change_notify: error parsing [path including drive letter to:]\theme.css: theme.css:8:99Failed to import: Operation not supported
The location points to the end of the filename of the first @ import url string.
This is caused by the string not being an url.
Based on suggestions from Jehan and lillolollo we replace g_file_get_path
with g_file_get_uri since an url is what is expected here.
Since that function already escapes the string we also remove
g_str_escape here.
In several GeglOperation filters, it is possible to pick a color. Up to
now, it was only possible to pick a color from the active layer (the one
you run the operation on). With this change, we can also pick in Sample
merged mode, same as Color Picker tool and other color tools.
The rational: advanced users won't really care about defaults (they know
to switch this option on/off depending on situation) but maybe beginners
would be less confused to pick "what they see" on first use, rather than
picking on the active layer? Now whatever is the default won't change
anyone's daily usage of GIMP. Clearly every image and use case is
different, so both with or without sample merged are useful one time or
another (this is why the option exists). It's really about the less
surprising option for beginners, based on usage statistics.
I ran a small poll on Twitter/Reddit/Patreon/Tipeee and ended up with
numbers of 131 for switching to "Sample merged" as default and 43
against, which is about 75% in favor. So let's just switch. It makes
sense anyway.
Make some of the bigger Preferences pages automatically scrollable if
needed. Based on my tests, this should be enough to fit on quite small
displays, at least with the default themes, even the 1366×768 reported
as too small. It should even fit in 1280×720 (in my tests, it did).
Targetting even smaller screens may be overdoing it for an image
manipulation software. We'll see if people still ask for a smaller
dialog.
Our Preferences exposes a concept of "Preferred color profile" (for RGB,
grayscale and CMYK), which is used in some places to be proposed as
default alternative to built-in profiles. But it was not used in the
import color profile dialog (only 2 choices were: keep the image profile
or convert to built-in RGB).
This commit now adds this third choice, which is even made default when
hitting the "Convert" button directly, without tweaking with the dialog.
Because we can assume that if someone made the explicit choice to label
such a profile as "Preferred", this is more likely the one to convert to
(if one even wants to convert from an embedded profile anyway).
As for the `Preferences > Image Import & Export > Color profile policy`,
they now propose 4 choices: Ask, Keep embedded profile, Convert to
built-in or preferred profiles.
