Page MenuHomePhabricator

elm_progressbar: set custom unit format if value is 0.0.
AbandonedPublic

Authored by netstar on Oct 22 2018, 4:55 AM.

Details

Reviewers
bu5hm4n
segfaultxavi
Group Reviewers
committers
Summary

If setting unit format for progressbar and the value
set with elm_progressbar_value_set is 0.0, the custom
unit format is broken and only a percentage is
displayed. This patch resolves this issue.

Test Plan
  • set custom format with progressbar using elm_progressbar_unit_format_set.
  • set progress bar value using elm_progressbar_value_set to 0.0.
  • The progress bar defaults to showing percent value string. Ignoring the custom format.
  • After applying this patch a value of 0.0 will result in proper custom setting of the unit format string for an elm_progressbar widget.

Diff Detail

Repository
rEFL core/efl
Branch
progress
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 7860
Build 7345: arc lint + arc unit
netstar created this revision.Oct 22 2018, 4:55 AM
netstar requested review of this revision.Oct 22 2018, 4:55 AM

Before:

After:

This should be backported. I can if it looks okay.

segfaultxavi requested changes to this revision.Oct 22 2018, 10:35 AM
segfaultxavi added a subscriber: segfaultxavi.

OK, so the problem is that the first refresh of the widget is skipped if the incoming value is the same as the one already set. In other words, the custom format is not displayed until the progress bar is set with a value different from the initial one.
In that case, checking for 0 does not seem right, since the initial value may not be 0. You need another mechanism to ensure that the first refresh is not skipped.

Out of curiosity, have you tried setting your custom format earlier on? Maybe inside the object constructor? Can you share that part of the code?

This revision now requires changes to proceed.Oct 22 2018, 10:35 AM

As far as I understand the unit format isn't a one-off call. It can be dynamic, as the progress bar itself is.

e.g. "88/101" or "74/eleventynine"

Will see if there's something better can be done.

OK, then changing the format should trigger an immediate refresh, instead of waiting for a change in the progress value.
(Maybe that's already there and it's failing for some other reason, I haven't looked at the code :p)

if (EINA_DBL_EQ(sd->val, val) &&
    (!EINA_DBL_EQ(sd->val, sd->val_min)) && (!EINA_DBL_EQ(sd->val, sd->val_max))) return;

???

You are still using the current value of the progress bar to detect if this is the first time the bar is drawn. What if I create a bar that starts at 50%?

The problem is elsewhere. I suspect setting a custom format should force a refresh of the bar which is not happening.
Can you share the code where this bar is created?

Hey,

It's not the first-run case.

This is called whenever elm_progressbar_value_set is called. Not the best naming I am sure slightly confusing. Anytime the cached value matches min/max in the range this is returning early and thus displaying "0%" rather than the custom string.

So you could be at 20% with "20 out of 40 potatoes" at the custom string, then the value is 0.0 and it'll display "0%" again each time.

Sorry my explanations were a bit lacking.

The custom display format is also dynamic like the progressbar meter.

progress = ui->progress_mem_shared;
elm_progressbar_unit_format_set(progress,
                                eina_slstr_printf(
                                "%lu %c / %lu %c",
                                _mem_adjust(ui->data_unit, results->memory.shared), *symbol,
                                _mem_adjust(ui->data_unit, results->memory.total), *symbol));
ratio = results->memory.total / 100.0;
value = results->memory.shared / ratio;
elm_progressbar_value_set(progress, value / 100);

This is called every N seconds. If value is 0.0 the custom string set in unit_format_set doesnt get rendered only "0%"

If the value being monitored, remains as 0.0 there is no rendering ever of the custom unit format.

I'm not sure this is solvable sanely. Or if it needs solving?

I was just firing shots in the dark, by the looks of the code around your change. After exploring a bit the whole context I think I'd better leave it to someone with more experience in this area. You'd have to bring in another reviewer, sorry :)

That's fine, no problem :)

netstar abandoned this revision.Nov 7 2018, 2:49 AM

Abandon this.