From a9e357d72dccc80ea7c99444bcc86ecfa68b1264 Mon Sep 17 00:00:00 2001 From: Ell Date: Mon, 6 Jan 2020 11:41:25 +0200 Subject: [PATCH] libgimpwidgets: fix percentage use in size-entry arithmetic when lower-bound != 0 In GimpSizeEntry, the value corresponding to 0%, as per gimp_size_entry_set_size(), may be non-zero. This works correctly when using the size entry in percentage mode, but not when using precentage as part of arithmetic. Fix this by adding an 'offset' parameter to eevl's unit-resolution callback, which can be specifies a constant value to add as part of unit conversion, after scaling the converted value by the conversion factor. In GimpSizeEntry, use this parameter to offset percentages by their lower bound. --- libgimpwidgets/gimpeevl.c | 28 +++++++++++++++++++++------ libgimpwidgets/gimpeevl.h | 6 ++++-- libgimpwidgets/gimpsizeentry.c | 35 +++++++++++++++++++++------------- libgimpwidgets/test-eevl.c | 13 ++++++++----- 4 files changed, 56 insertions(+), 26 deletions(-) diff --git a/libgimpwidgets/gimpeevl.c b/libgimpwidgets/gimpeevl.c index 4f899ce91f..55bf3232bb 100644 --- a/libgimpwidgets/gimpeevl.c +++ b/libgimpwidgets/gimpeevl.c @@ -237,6 +237,7 @@ gimp_eevl_complete (GimpEevl *eva) { GimpEevlQuantity result = {0, 0}; GimpEevlQuantity default_unit_factor; + gdouble default_unit_offset; /* Empty expression evaluates to 0 */ if (gimp_eevl_accept (eva, GIMP_EEVL_TOKEN_END, NULL)) @@ -249,6 +250,7 @@ gimp_eevl_complete (GimpEevl *eva) eva->options.unit_resolver_proc (NULL, &default_unit_factor, + &default_unit_offset, eva->options.data); /* Entire expression is dimensionless, apply default unit if @@ -257,6 +259,7 @@ gimp_eevl_complete (GimpEevl *eva) if (result.dimension == 0 && default_unit_factor.dimension != 0) { result.value /= default_unit_factor.value; + result.value += default_unit_offset; result.dimension = default_unit_factor.dimension; } return result; @@ -282,21 +285,25 @@ gimp_eevl_expression (GimpEevl *eva) if (new_term.dimension != evaluated_terms.dimension) { GimpEevlQuantity default_unit_factor; + gdouble default_unit_offset; eva->options.unit_resolver_proc (NULL, &default_unit_factor, + &default_unit_offset, eva->options.data); if (new_term.dimension == 0 && evaluated_terms.dimension == default_unit_factor.dimension) { new_term.value /= default_unit_factor.value; + new_term.value += default_unit_offset; new_term.dimension = default_unit_factor.dimension; } else if (evaluated_terms.dimension == 0 && new_term.dimension == default_unit_factor.dimension) { evaluated_terms.value /= default_unit_factor.value; + evaluated_terms.value += default_unit_offset; evaluated_terms.dimension = default_unit_factor.dimension; } else @@ -438,7 +445,8 @@ gimp_eevl_quantity (GimpEevl *eva) if (eva->current_token.type == GIMP_EEVL_TOKEN_IDENTIFIER) { gchar *identifier; - GimpEevlQuantity result; + GimpEevlQuantity factor; + gdouble offset; gimp_eevl_accept (eva, GIMP_EEVL_TOKEN_ANY, @@ -450,7 +458,8 @@ gimp_eevl_quantity (GimpEevl *eva) identifier[consumed_token.value.size] = '\0'; if (eva->options.unit_resolver_proc (identifier, - &result, + &factor, + &offset, eva->options.data)) { if (gimp_eevl_accept (eva, '^', NULL)) @@ -465,12 +474,19 @@ gimp_eevl_quantity (GimpEevl *eva) "Exponent is not a dimensionless quantity"); } - result.value = pow (result.value, evaluated_exponent.value); - result.dimension *= evaluated_exponent.value; + if (offset != 0.0) + { + gimp_eevl_error (eva, + "Invalid unit exponent"); + } + + factor.value = pow (factor.value, evaluated_exponent.value); + factor.dimension *= evaluated_exponent.value; } - evaluated_quantity.value /= result.value; - evaluated_quantity.dimension += result.dimension; + evaluated_quantity.value /= factor.value; + evaluated_quantity.value += offset; + evaluated_quantity.dimension += factor.dimension; } else { diff --git a/libgimpwidgets/gimpeevl.h b/libgimpwidgets/gimpeevl.h index e826f4ed9d..bbcb69bdc5 100644 --- a/libgimpwidgets/gimpeevl.h +++ b/libgimpwidgets/gimpeevl.h @@ -42,16 +42,18 @@ typedef struct * GimpEevlUnitResolverProc: * @identifier: Identifier of unit to resolve or %NULL if default unit * should be resolved. - * @result: Units per reference unit. For example, in GIMP the + * @factor: Units per reference unit. For example, in GIMP the * reference unit is inches so resolving "mm" should * return 25.4 since there are 25.4 millimeters per inch. + * @offset: Offset to apply after scaling the value according to @factor. * @data: Data given to gimp_eevl_evaluate(). * * Returns: If the unit was successfully resolved or not. * */ typedef gboolean (* GimpEevlUnitResolverProc) (const gchar *identifier, - GimpEevlQuantity *result, + GimpEevlQuantity *factor, + gdouble *offset, gpointer data); diff --git a/libgimpwidgets/gimpsizeentry.c b/libgimpwidgets/gimpsizeentry.c index 0edeb392b2..aea53f72f1 100644 --- a/libgimpwidgets/gimpsizeentry.c +++ b/libgimpwidgets/gimpsizeentry.c @@ -139,7 +139,8 @@ static gint gimp_size_entry_eevl_input_callback (GtkSpinButton *spinne gdouble *return_val, gpointer *data); static gboolean gimp_size_entry_eevl_unit_resolver (const gchar *ident, - GimpEevlQuantity *result, + GimpEevlQuantity *factor, + gdouble *offset, gpointer data); @@ -1383,6 +1384,7 @@ gimp_size_entry_eevl_input_callback (GtkSpinButton *spinner, { GimpSizeEntryField *other_gsef; GimpEevlQuantity default_unit_factor; + gdouble default_unit_offset; options.ratio_expressions = TRUE; @@ -1399,7 +1401,9 @@ gimp_size_entry_eevl_input_callback (GtkSpinButton *spinner, options.ratio_invert = TRUE; } - options.unit_resolver_proc (NULL, &default_unit_factor, options.data); + options.unit_resolver_proc (NULL, + &default_unit_factor, &default_unit_offset, + options.data); options.ratio_quantity.value = other_gsef->value / default_unit_factor.value; @@ -1488,7 +1492,8 @@ gimp_size_entry_eevl_input_callback (GtkSpinButton *spinner, static gboolean gimp_size_entry_eevl_unit_resolver (const gchar *identifier, - GimpEevlQuantity *result, + GimpEevlQuantity *factor, + gdouble *offset, gpointer data) { GimpSizeEntryField *gsef = (GimpSizeEntryField *) data; @@ -1497,9 +1502,11 @@ gimp_size_entry_eevl_unit_resolver (const gchar *identifier, GimpUnit unit; g_return_val_if_fail (gsef, FALSE); - g_return_val_if_fail (result != NULL, FALSE); + g_return_val_if_fail (factor != NULL, FALSE); + g_return_val_if_fail (offset != NULL, FALSE); g_return_val_if_fail (GIMP_IS_SIZE_ENTRY (gsef->gse), FALSE); + *offset = 0.0; for (unit = 0; unit <= gimp_unit_get_number_of_units (); @@ -1519,34 +1526,36 @@ gimp_size_entry_eevl_unit_resolver (const gchar *identifier, case GIMP_UNIT_PERCENT: if (priv->unit == GIMP_UNIT_PERCENT) { - result->value = 1; - result->dimension = 0; + factor->value = 1; + factor->dimension = 0; } else { /* gsef->upper contains the '100%'-value */ - result->value = 100*gsef->resolution/gsef->upper; - result->dimension = 1; + factor->value = 100*gsef->resolution/(gsef->upper - gsef->lower); + /* gsef->lower contains the '0%'-value */ + *offset = gsef->lower/gsef->resolution; + factor->dimension = 1; } /* return here, don't perform percentage conversion */ return TRUE; case GIMP_UNIT_PIXEL: - result->value = gsef->resolution; + factor->value = gsef->resolution; break; default: - result->value = gimp_unit_get_factor (unit); + factor->value = gimp_unit_get_factor (unit); break; } if (priv->unit == GIMP_UNIT_PERCENT) { /* map non-percentages onto percent */ - result->value = gsef->upper/(100*gsef->resolution); - result->dimension = 0; + factor->value = gsef->upper/(100*gsef->resolution); + factor->dimension = 0; } else { - result->dimension = 1; + factor->dimension = 1; } /* We are done */ diff --git a/libgimpwidgets/test-eevl.c b/libgimpwidgets/test-eevl.c index fb3a40ce0e..7c5b9c02dd 100644 --- a/libgimpwidgets/test-eevl.c +++ b/libgimpwidgets/test-eevl.c @@ -42,24 +42,27 @@ typedef struct static gboolean test_units (const gchar *ident, - GimpEevlQuantity *result, + GimpEevlQuantity *factor, + gdouble *offset, gpointer data) { gboolean resolved = FALSE; gboolean default_unit = (ident == NULL); + *offset = 0.0; + if (default_unit || (ident && strcmp ("in", ident) == 0)) { - result->dimension = 1; - result->value = 1.; + factor->dimension = 1; + factor->value = 1.; resolved = TRUE; } else if (ident && strcmp ("mm", ident) == 0) { - result->dimension = 1; - result->value = 25.4; + factor->dimension = 1; + factor->value = 25.4; resolved = TRUE; }