Page MenuHomePhabricator

slider: fix value error from step
ClosedPublic

Authored by bowonryu on Nov 14 2019, 1:03 AM.

Details

Summary

When the slider moves using step,
_drag_value_fetch(), _val_fetch() calculates a value from position of edje_part.
Then the calculated value is updated.
However, this causes a slight error.

This patch updates value ​​first when moving with steps.

  • Test Example
Evas_Object *sl = elm_slider_add(bx);
elm_slider_min_max_set(sl, -5, 5);
elm_slider_value_set(sl, 0.0);
elm_slider_step_set(sl, 0.1);
evas_object_size_hint_align_set(sl, EVAS_HINT_FILL, EVAS_HINT_FILL);
evas_object_size_hint_weight_set(sl, EVAS_HINT_EXPAND, EVAS_HINT_EXPAND);
evas_object_smart_callback_add(sl, "changed", _change_cb, NULL);
void
_change_cb(void *data, Evas_Object *obj, void *event_info EINA_UNUSED)
{
   double val = elm_slider_value_get(obj);

   if (val == -5.0) printf("val[%f] == -5.0 \n", val);
   if (val == -4.0) printf("val[%f] == -4.0 \n", val);
   if (val == -3.0) printf("val[%f] == -3.0 \n", val);
   if (val == -2.0) printf("val[%f] == -2.0 \n", val);
   if (val == -1.0) printf("val[%f] == -1.0 \n", val);
   if (val == 0.0) printf("val[%f] == 0.0 \n", val);
   if (val == 1.0) printf("val[%f] == 1.0 \n", val);
   if (val == 2.0) printf("val[%f] == 2.0 \n", val);
   if (val == 3.0) printf("val[%f] == 3.0 \n", val);
   if (val == 4.0) printf("val[%f] == 4.0 \n", val);
   if (val == 5.0) printf("val[%f] == 5.0 \n", val);
}

If you move the slider using step in this test,
You can see that some logs are not visible. (Some values ​​are incorrect)

Test Plan

elementary_test -to slider
elementary_test -to efl.ui.slider

Diff Detail

Repository
rEFL core/efl
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
bowonryu created this revision.Nov 14 2019, 1:03 AM
bowonryu requested review of this revision.Nov 14 2019, 1:03 AM
bowonryu updated this revision to Diff 26886.Nov 14 2019, 1:11 AM

remove white space

bowonryu updated this revision to Diff 26887.Nov 14 2019, 1:14 AM

remove white spaces.

bowonryu updated this revision to Diff 26916.Nov 14 2019, 9:04 PM

add missing conditions.

bowonryu updated this revision to Diff 26917.Nov 14 2019, 9:44 PM

remove white space

Can you elaborate in the commit message what exactly is the issue this is fixing ?

bowonryu updated this revision to Diff 26924.Nov 14 2019, 11:34 PM

update missing code

woohyun accepted this revision.Nov 15 2019, 12:07 AM
This revision is now accepted and ready to land.Nov 15 2019, 12:07 AM
bowonryu updated this revision to Diff 26925.Nov 15 2019, 12:08 AM

logic reordering

bowonryu updated this revision to Diff 26926.Nov 15 2019, 1:01 AM
bowonryu edited the summary of this revision. (Show Details)

update example

bu5hm4n requested changes to this revision.Nov 15 2019, 1:10 AM

Okay, i now see what you want to fix. Thank you for the fix & the explanation.

However, i would be happy if the duplication here would be reduced, and the code could be a bit more compact. (The CLAP macro will help there).

(Can you plan to add some test for this in efl_ui_test_slider.c ? there is already one for dragging with the mouse ... :))

src/lib/elementary/efl_ui_slider.c
218

You can use something like value = CLAMP(sd->val + step, sd->val_max [...]) which makes the whole thing lesser code.

241

This complete block is duplicated.

src/lib/elementary/elm_slider.c
535

This complete block is duplicated.

555

This complete block is duplicated.

This revision now requires changes to proceed.Nov 15 2019, 1:10 AM

Oh - i dont mind of the duplicated blocks are in both files. But they should *not* be repeated within the same file ... :)

bowonryu updated this revision to Diff 26927.Nov 15 2019, 1:48 AM

using CLAMP macro to reduce duplicate code.

Thank you for the update, however I would still prefer to have the 4 lines refactored into a single function, and not copy and pasted across the same file :)

bowonryu updated this revision to Diff 26956.Nov 17 2019, 8:51 PM

refactored into a single function for reduce duplicate code.

bu5hm4n accepted this revision.Nov 17 2019, 11:09 PM

Thank you :)

This revision is now accepted and ready to land.Nov 17 2019, 11:09 PM
This revision was automatically updated to reflect the committed changes.