From 978206dee4d685cdb37034210dc8cc78389eb777 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rom=C3=A1n=20Mart=C3=ADnez?= Date: Sat, 3 Apr 2021 16:32:53 +0200 Subject: [PATCH] IntervalsFragment/IntervalStatisticsModel refactoring: better performance with AndroidViewModel (it doesn't recreate the model every track point done anymore). Also, it doesn't jump anymore. Fixes #669. --- .../opentracks/TrackRecordedActivity.java | 2 +- .../opentracks/TrackRecordingActivity.java | 2 +- .../adapters/IntervalStatisticsAdapter.java | 109 +++++++++------- .../fragments/IntervalsFragment.java | 116 ++++++++---------- .../viewmodels/IntervalStatistics.java | 1 - .../viewmodels/IntervalStatisticsModel.java | 53 ++++++-- src/main/res/layout/interval_list_view.xml | 2 +- 7 files changed, 162 insertions(+), 123 deletions(-) diff --git a/src/main/java/de/dennisguse/opentracks/TrackRecordedActivity.java b/src/main/java/de/dennisguse/opentracks/TrackRecordedActivity.java index 7e1cc1141..1ce3869d9 100644 --- a/src/main/java/de/dennisguse/opentracks/TrackRecordedActivity.java +++ b/src/main/java/de/dennisguse/opentracks/TrackRecordedActivity.java @@ -268,7 +268,7 @@ public class TrackRecordedActivity extends AbstractListActivity implements Confi case 0: return StatisticsRecordedFragment.newInstance(trackId); case 1: - return IntervalsFragment.newInstance(); + return IntervalsFragment.newInstance(true); case 2: return ChartFragment.newInstance(false); case 3: diff --git a/src/main/java/de/dennisguse/opentracks/TrackRecordingActivity.java b/src/main/java/de/dennisguse/opentracks/TrackRecordingActivity.java index c06769276..46b8f5b53 100644 --- a/src/main/java/de/dennisguse/opentracks/TrackRecordingActivity.java +++ b/src/main/java/de/dennisguse/opentracks/TrackRecordingActivity.java @@ -369,7 +369,7 @@ public class TrackRecordingActivity extends AbstractActivity implements ChooseAc case 0: return StatisticsRecordingFragment.newInstance(); case 1: - return IntervalsFragment.IntervalsRecordingFragment.newInstance(); + return IntervalsFragment.newInstance(false); case 2: return ChartFragment.newInstance(false); case 3: diff --git a/src/main/java/de/dennisguse/opentracks/adapters/IntervalStatisticsAdapter.java b/src/main/java/de/dennisguse/opentracks/adapters/IntervalStatisticsAdapter.java index 3c38b4ac4..e782a062b 100644 --- a/src/main/java/de/dennisguse/opentracks/adapters/IntervalStatisticsAdapter.java +++ b/src/main/java/de/dennisguse/opentracks/adapters/IntervalStatisticsAdapter.java @@ -4,73 +4,84 @@ import android.content.Context; import android.view.LayoutInflater; import android.view.View; import android.view.ViewGroup; -import android.widget.ArrayAdapter; import android.widget.TextView; import androidx.annotation.NonNull; -import androidx.annotation.Nullable; +import androidx.recyclerview.widget.RecyclerView; import java.util.List; import de.dennisguse.opentracks.R; -import de.dennisguse.opentracks.util.PreferencesUtils; import de.dennisguse.opentracks.util.StringUtils; import de.dennisguse.opentracks.viewmodels.IntervalStatistics; -public class IntervalStatisticsAdapter extends ArrayAdapter { +public class IntervalStatisticsAdapter extends RecyclerView.Adapter { + private List intervalList; + private final Context context; private final StackMode stackMode; - private final boolean metricUnits; - private final String category; + private boolean metricUnits; + private boolean isReportSpeed; - public IntervalStatisticsAdapter(Context context, List intervalList, String category, StackMode stackMode) { - super(context, R.layout.interval_stats_list_item, intervalList); - metricUnits = PreferencesUtils.isMetricUnits(PreferencesUtils.getSharedPreferences(context), context); - this.category = category; + public IntervalStatisticsAdapter(Context context, StackMode stackMode, boolean metricUnits, boolean isReportSpeed) { + this.metricUnits = metricUnits; + this.context = context; this.stackMode = stackMode; + this.isReportSpeed = isReportSpeed; } - //TODO Check preference handling! Should not be accessed in getView() @NonNull @Override - public View getView(int position, @Nullable View intervalView, @NonNull ViewGroup parent) { - int actualPosition = stackMode == StackMode.STACK_FROM_TOP ? position : getCount() - 1 - position; - IntervalStatistics.Interval interval = getItem(actualPosition); - ViewHolder viewHolder; + public RecyclerView.ViewHolder onCreateViewHolder(@NonNull ViewGroup parent, int viewType) { + View view = LayoutInflater.from(parent.getContext()).inflate(R.layout.interval_stats_list_item, parent, false); + return new IntervalStatisticsAdapter.ViewHolder(view); + } - if (intervalView == null) { - viewHolder = new ViewHolder(); - - intervalView = LayoutInflater.from(getContext()).inflate(R.layout.interval_stats_list_item, parent, false); - - viewHolder.distance = intervalView.findViewById(R.id.interval_item_distance); - viewHolder.rate = intervalView.findViewById(R.id.interval_item_rate); - viewHolder.gain = intervalView.findViewById(R.id.interval_item_gain); - viewHolder.loss = intervalView.findViewById(R.id.interval_item_loss); - - intervalView.setTag(viewHolder); - } else { - viewHolder = (ViewHolder) intervalView.getTag(); - } + @Override + public void onBindViewHolder(@NonNull RecyclerView.ViewHolder holder, int position) { + int actualPosition = stackMode == StackMode.STACK_FROM_TOP ? position : getItemCount() - 1 - position; + int nextPosition = actualPosition + 1; + boolean isLast = actualPosition == getItemCount() - 1; + IntervalStatisticsAdapter.ViewHolder viewHolder = (IntervalStatisticsAdapter.ViewHolder) holder; + IntervalStatistics.Interval interval = intervalList.get(actualPosition); + viewHolder.itemView.setTag(actualPosition); float sumDistance_m; - if (actualPosition + 1 == getCount() && actualPosition > 0) { - sumDistance_m = actualPosition * getItem(actualPosition - 1).getDistance_m() + interval.getDistance_m(); + if (isLast && actualPosition > 0) { + sumDistance_m = actualPosition * intervalList.get(actualPosition - 1).getDistance_m() + interval.getDistance_m(); } else { - sumDistance_m = (actualPosition + 1) * interval.getDistance_m(); + sumDistance_m = nextPosition * interval.getDistance_m(); } - viewHolder.distance.setText(StringUtils.formatDistance(getContext(), sumDistance_m, metricUnits)); + viewHolder.distance.setText(StringUtils.formatDistance(context, sumDistance_m, metricUnits)); - if (PreferencesUtils.isReportSpeed(PreferencesUtils.getSharedPreferences(getContext()), getContext(), category)) { - viewHolder.rate.setText(StringUtils.formatSpeed(getContext(), interval.getSpeed_ms(), metricUnits, true)); - } else { - viewHolder.rate.setText(StringUtils.formatSpeed(getContext(), interval.getSpeed_ms(), metricUnits, false)); + viewHolder.rate.setText(StringUtils.formatSpeed(context, interval.getSpeed_ms(), metricUnits, isReportSpeed)); + + viewHolder.gain.setText(StringUtils.formatDistance(context, interval.getGain_m(), metricUnits)); + viewHolder.loss.setText(StringUtils.formatDistance(context, interval.getLoss_m(), metricUnits)); + } + + @Override + public int getItemCount() { + if (intervalList == null) { + return 0; + } + return intervalList.size(); + } + + public List swapData(List data, boolean metricUnits, boolean isReportSpeed) { + if (intervalList == data && this.metricUnits == metricUnits && this.isReportSpeed == isReportSpeed) { + return null; } - viewHolder.gain.setText(StringUtils.formatDistance(getContext(), interval.getGain_m(), metricUnits)); - viewHolder.loss.setText(StringUtils.formatDistance(getContext(), interval.getLoss_m(), metricUnits)); + this.metricUnits = metricUnits; + this.isReportSpeed = isReportSpeed; + intervalList = data; - return intervalView; + if (data != null) { + this.notifyDataSetChanged(); + } + + return data; } /** @@ -81,10 +92,18 @@ public class IntervalStatisticsAdapter extends ArrayAdapter { if (PreferencesUtils.isKey(getContext(), R.string.stats_units_key, key) || PreferencesUtils.isKey(getContext(), R.string.stats_rate_key, key)) { metricUnits = PreferencesUtils.isMetricUnits(sharedPreferences, getContext()); - if (adapter != null) { - adapter.notifyDataSetChanged(); + uploadIntervals(); + if (spinnerAdapter != null) { spinnerAdapter.notifyDataSetChanged(); } } }; - public static Fragment newInstance() { - return new IntervalsFragment(); + /** + * Creates an instance of this class. + * + * @param fromTopToBottom If true then the intervals are shown from top to bottom (the first interval on top). Otherwise the intervals are shown from bottom to top. + * @return IntervalsFragment instance. + */ + public static Fragment newInstance(boolean fromTopToBottom) { + Bundle bundle = new Bundle(); + bundle.putBoolean(FROM_TOP_TO_BOTTOM_KEY, fromTopToBottom); + IntervalsFragment intervalsFragment = new IntervalsFragment(); + intervalsFragment.setArguments(bundle); + return intervalsFragment; + } + + @Override + public void onCreate(@Nullable Bundle savedInstanceState) { + super.onCreate(savedInstanceState); + stackModeListView = getArguments().getBoolean(FROM_TOP_TO_BOTTOM_KEY, true) ? IntervalStatisticsAdapter.StackMode.STACK_FROM_TOP : IntervalStatisticsAdapter.StackMode.STACK_FROM_BOTTOM; } @Override @@ -76,14 +97,11 @@ public class IntervalsFragment extends Fragment implements TrackDataListener { super.onViewCreated(view, savedInstanceState); sharedPreferences = PreferencesUtils.getSharedPreferences(getContext()); - sharedPreferences.registerOnSharedPreferenceChangeListener(sharedPreferenceChangeListener); - sharedPreferenceChangeListener.onSharedPreferenceChanged(sharedPreferences, null); - viewBinding.intervalList.setEmptyView(viewBinding.intervalListEmptyView); - - stackModeListView = IntervalStatisticsAdapter.StackMode.STACK_FROM_TOP; - - viewModel = new IntervalStatisticsModel(); + adapter = new IntervalStatisticsAdapter(getContext(), stackModeListView, metricUnits, isReportSpeed); + viewBinding.intervalList.setLayoutManager(new LinearLayoutManager(getContext())); + // TODO handle empty view: before we did viewBinding.intervalList.setEmptyView(viewBinding.intervalListEmptyView); + viewBinding.intervalList.setAdapter(adapter); spinnerAdapter = new ArrayAdapter(getContext(), android.R.layout.simple_spinner_dropdown_item, IntervalStatisticsModel.IntervalOption.values()) { @NonNull @@ -109,7 +127,7 @@ public class IntervalsFragment extends Fragment implements TrackDataListener { @Override public void onItemSelected(AdapterView adapterView, View view, int i, long l) { selectedInterval = IntervalStatisticsModel.IntervalOption.values()[i]; - loadIntervals(); + uploadIntervals(); } @Override @@ -121,6 +139,10 @@ public class IntervalsFragment extends Fragment implements TrackDataListener { @Override public void onResume() { super.onResume(); + sharedPreferences.registerOnSharedPreferenceChangeListener(sharedPreferenceChangeListener); + sharedPreferenceChangeListener.onSharedPreferenceChanged(sharedPreferences, null); + viewModel = new ViewModelProvider(getActivity()).get(IntervalStatisticsModel.class); + loadIntervals(); resumeTrackDataHub(); } @@ -128,7 +150,6 @@ public class IntervalsFragment extends Fragment implements TrackDataListener { public void onPause() { super.onPause(); pauseTrackDataHub(); - sharedPreferences.unregisterOnSharedPreferenceChangeListener(sharedPreferenceChangeListener); } @@ -155,10 +176,15 @@ public class IntervalsFragment extends Fragment implements TrackDataListener { if (viewModel == null) { return; } + LiveData> liveData = viewModel.getIntervalStats(metricUnits, selectedInterval); + liveData.observe(getActivity(), intervalList -> adapter.swapData(intervalList, metricUnits, isReportSpeed)); + } - IntervalStatistics intervalStatistics = viewModel.getIntervalStats(metricUnits, selectedInterval); - adapter = new IntervalStatisticsAdapter(getContext(), intervalStatistics.getIntervalList(), category, stackModeListView); - viewBinding.intervalList.setAdapter(adapter); + private synchronized void uploadIntervals() { + if (viewModel == null) { + return; + } + viewModel.upload(metricUnits, selectedInterval); } /** @@ -185,11 +211,11 @@ public class IntervalsFragment extends Fragment implements TrackDataListener { getActivity().runOnUiThread(() -> { if (isResumed()) { // Set category. - category = track != null ? track.getCategory() : ""; + String category = track != null ? track.getCategory() : ""; // Set rate label. - boolean reportSpeed = PreferencesUtils.isReportSpeed(sharedPreferences, getContext(), category); //TODO Handle sharedPreferenceChangeListener - viewBinding.intervalRate.setText(reportSpeed ? R.string.stats_speed : R.string.stats_pace); + isReportSpeed = PreferencesUtils.isReportSpeed(sharedPreferences, getContext(), category); + viewBinding.intervalRate.setText(isReportSpeed ? R.string.stats_speed : R.string.stats_pace); } }); } @@ -219,7 +245,7 @@ public class IntervalsFragment extends Fragment implements TrackDataListener { @Override public void onNewTrackPointsDone(@NonNull TrackPoint unused) { if (isResumed()) { - runOnUiThread(this::loadIntervals); + runOnUiThread(viewModel::onNewTrackPoints); } } @@ -249,48 +275,4 @@ public class IntervalsFragment extends Fragment implements TrackDataListener { fragmentActivity.runOnUiThread(runnable); } } - - public static class IntervalsRecordingFragment extends IntervalsFragment { - // Refreshing intervals stats it's not so demanding so 5 seconds is enough to balance performance and user experience. - private static final long UI_UPDATE_INTERVAL = 5 * UnitConversions.ONE_SECOND_MS; - - private Handler intervalHandler; - - private final Runnable intervalRunner = new Runnable() { - @Override - public void run() { - if (isResumed()) { - updateIntervals(); - intervalHandler.postDelayed(intervalRunner, UI_UPDATE_INTERVAL); - } - } - }; - - public static Fragment newInstance() { - return new IntervalsRecordingFragment(); - } - - @Override - public void onViewCreated(@NonNull View view, @Nullable Bundle savedInstanceState) { - super.onViewCreated(view, savedInstanceState); - intervalHandler = new Handler(); - stackModeListView = IntervalStatisticsAdapter.StackMode.STACK_FROM_BOTTOM; - } - - @Override - public void onResume() { - super.onResume(); - intervalHandler.post(intervalRunner); - } - - @Override - public void onPause() { - super.onPause(); - intervalHandler.removeCallbacks(intervalRunner); - } - - private void updateIntervals() { - loadIntervals(); - } - } } diff --git a/src/main/java/de/dennisguse/opentracks/viewmodels/IntervalStatistics.java b/src/main/java/de/dennisguse/opentracks/viewmodels/IntervalStatistics.java index d5be00242..bc44d72f0 100644 --- a/src/main/java/de/dennisguse/opentracks/viewmodels/IntervalStatistics.java +++ b/src/main/java/de/dennisguse/opentracks/viewmodels/IntervalStatistics.java @@ -55,7 +55,6 @@ public class IntervalStatistics { } } - public List getIntervalList() { return intervalList; } diff --git a/src/main/java/de/dennisguse/opentracks/viewmodels/IntervalStatisticsModel.java b/src/main/java/de/dennisguse/opentracks/viewmodels/IntervalStatisticsModel.java index 459a8e887..e79fbc752 100644 --- a/src/main/java/de/dennisguse/opentracks/viewmodels/IntervalStatisticsModel.java +++ b/src/main/java/de/dennisguse/opentracks/viewmodels/IntervalStatisticsModel.java @@ -1,6 +1,11 @@ package de.dennisguse.opentracks.viewmodels; +import android.app.Application; + +import androidx.annotation.NonNull; import androidx.annotation.Nullable; +import androidx.lifecycle.AndroidViewModel; +import androidx.lifecycle.MutableLiveData; import java.util.ArrayList; import java.util.List; @@ -12,33 +17,67 @@ import de.dennisguse.opentracks.util.UnitConversions; * This model is used to load intervals for a track. * It uses a default interval but it can be set from outside to manage the interval length. */ -public class IntervalStatisticsModel { +public class IntervalStatisticsModel extends AndroidViewModel { private final List trackPoints = new ArrayList<>(); + private MutableLiveData> intervalsLiveData; + private float distanceInterval; - public IntervalStatistics getIntervalStats(boolean metricUnits, @Nullable IntervalOption interval) { + public IntervalStatisticsModel(@NonNull Application application) { + super(application); + } + + public MutableLiveData> getIntervalStats(boolean metricUnits, @Nullable IntervalOption interval) { synchronized (trackPoints) { - if (interval == null) { - interval = IntervalOption.OPTION_1; - } + if (intervalsLiveData == null) { + if (interval == null) { + interval = IntervalOption.OPTION_1; + } - float distanceInterval = metricUnits ? (float) (interval.getValue() * UnitConversions.KM_TO_M) : (float) (interval.getValue() * UnitConversions.MI_TO_M); - return new IntervalStatistics(trackPoints, distanceInterval); + intervalsLiveData = new MutableLiveData<>(); + distanceInterval = metricUnits ? (float) (interval.getValue() * UnitConversions.KM_TO_M) : (float) (interval.getValue() * UnitConversions.MI_TO_M); + loadIntervalStatistics(); + } + return intervalsLiveData; } } + private void loadIntervalStatistics() { + IntervalStatistics intervalStatistics = new IntervalStatistics(trackPoints, distanceInterval); + intervalsLiveData.postValue(intervalStatistics.getIntervalList()); + } + public void add(TrackPoint trackPoint) { synchronized (trackPoints) { trackPoints.add(trackPoint); } } + public void onNewTrackPoints() { + synchronized (trackPoints) { + if (intervalsLiveData != null) { + loadIntervalStatistics(); + } + } + } + public void clear() { synchronized (trackPoints) { trackPoints.clear(); } } + public void upload(boolean metricUnits, @Nullable IntervalOption interval) { + synchronized (trackPoints) { + if (interval == null) { + interval = IntervalOption.OPTION_1; + } + + distanceInterval = metricUnits ? (float) (interval.getValue() * UnitConversions.KM_TO_M) : (float) (interval.getValue() * UnitConversions.MI_TO_M); + loadIntervalStatistics(); + } + } + /** * Intervals length this view model support. */ diff --git a/src/main/res/layout/interval_list_view.xml b/src/main/res/layout/interval_list_view.xml index 6e58ba1c0..cc2e9ab9b 100644 --- a/src/main/res/layout/interval_list_view.xml +++ b/src/main/res/layout/interval_list_view.xml @@ -57,7 +57,7 @@ android:layout_marginEnd="8dp" android:background="@color/stats_separator" /> -