From 012020735f5c12f41765877cea6d5e3673ccd91e Mon Sep 17 00:00:00 2001 From: Jason-Whitmore Date: Sat, 10 May 2025 04:04:57 -0700 Subject: [PATCH] Fixes Issue 6262: [Bug]: Error occurred while loading images (#6291) * ImageInfo.kt: remove call to updateThumbURL() Before this commit, the updateThumbURL() method would attempt to derive a larger resolution thumbnail URL from the current thumbnail's width and height. This derived thumbnail URL would sometimes not match an actual URL. Even creating this thumbnail URL would sometimes fail when doing string manipulation. Both errors can cause issues, with the second one crashing the thread and preventing images from being loaded. This commit simply removes the call to updateThumbURL(). Images with their thumbnails now load correctly, especially with panoramic images. * MediaInterface.kt: fix MediaWiki API call to retrieve thumbnail URLs properly Before this change, the API calls to MediaWiki would request thumbnails with atleast a specific width, rather than height. These thumbnails would often be extremely low resolution on very wide images, such as panoramas. This change instead requests thumbnails with atleast a specific height. Panorama thumbnails are now at a higher resolution than before. Additionally, the height parameter is now represented as an integer which can be changed more easily. * ViewPagerAdapter.java: create new constructor with behavior parameter Before this commit, ViewPagerAdapter only had one constructor which used the default behavior where more than the current fragment would be in the Resume state. This commit adds a constructor where the behavior parameter can be specified. * ExploreFragment.java: modify onCreateView to use new ViewPagerAdapter constructor Before this change, onCreateView would use the old ViewPagerAdapter constructor, which had a default behavior where fragments not currently visible would be in the Resume state. After this change, the new ViewPagerAdapter constructor is used with a non-default behavior where only the visible fragment would be in the Resume state. This has performance benefits for most users since the Fragments will only be active when they want to view them. * ExploreMapFragment.java: remove redundant method call Before this commit, performMapReadyActions() was called in both onViewCreated and onResume. In effect, the method is called twice before the user can even see the map, leading to performance issues. After this commit, the call in onViewCreated is removed. The method is now called only once when the user switches to the ExploreMapFragment. * ExploreMapFragment.java: remove method call that causes recursive loop Before this commit, there was a call loop that included onScroll and animateTo on the map. These two method calls caused performance issues in ExploreMapFragment. This commit breaks the call loop by removing moveCameraToPosition from getMapCenter. Also, the removed code did not fit the purpose of getMapCenter. * ExploreMapFragment.java: fix performMapReadyActions to center to user's last known location Before this commit, fixing a previous bug caused performMapReadyActions to center the map on a default location rather than the available last known location. This commit adds code which will attempt to get the last known user location. If it cannot be retrieved, the default location is used. The map now centers properly. * MediaInterface.kt: adjust thumbnail height parameter Before this commit, the thumbnail height parameter was too small, causing low resolution thumbnails to render. This commit more than doubles the parameter, increasing the thumbnail resolution. --------- Co-authored-by: Ritika Pahwa <83745993+RitikaPahwa4444@users.noreply.github.com> Co-authored-by: Nicolas Raoul --- .../fr/free/nrw/commons/ViewPagerAdapter.java | 11 +++++++++++ .../nrw/commons/explore/ExploreFragment.java | 5 ++++- .../commons/explore/map/ExploreMapFragment.java | 11 ++++++----- .../fr/free/nrw/commons/media/MediaInterface.kt | 16 ++++++++++++++-- .../commons/wikidata/model/gallery/ImageInfo.kt | 1 - 5 files changed, 35 insertions(+), 9 deletions(-) diff --git a/app/src/main/java/fr/free/nrw/commons/ViewPagerAdapter.java b/app/src/main/java/fr/free/nrw/commons/ViewPagerAdapter.java index 5ca20372a..b887aaf99 100644 --- a/app/src/main/java/fr/free/nrw/commons/ViewPagerAdapter.java +++ b/app/src/main/java/fr/free/nrw/commons/ViewPagerAdapter.java @@ -18,6 +18,17 @@ public class ViewPagerAdapter extends FragmentPagerAdapter { super(manager); } + /** + * Constructs a ViewPagerAdapter with a specified Fragment Manager and Fragment resume behavior. + * + * @param manager The FragmentManager + * @param behavior An integer which represents the behavior of non visible fragments. See + * FragmentPagerAdapter.java for options. + */ + public ViewPagerAdapter(FragmentManager manager, int behavior) { + super(manager, behavior); + } + /** * This method returns the fragment of the viewpager at a particular position * @param position diff --git a/app/src/main/java/fr/free/nrw/commons/explore/ExploreFragment.java b/app/src/main/java/fr/free/nrw/commons/explore/ExploreFragment.java index 223d028dc..b31c34b67 100644 --- a/app/src/main/java/fr/free/nrw/commons/explore/ExploreFragment.java +++ b/app/src/main/java/fr/free/nrw/commons/explore/ExploreFragment.java @@ -12,6 +12,7 @@ import android.view.ViewGroup; import androidx.annotation.NonNull; import androidx.annotation.Nullable; import androidx.fragment.app.Fragment; +import androidx.fragment.app.FragmentPagerAdapter; import androidx.viewpager.widget.ViewPager.OnPageChangeListener; import fr.free.nrw.commons.R; import fr.free.nrw.commons.ViewPagerAdapter; @@ -69,7 +70,9 @@ public class ExploreFragment extends CommonsDaggerSupportFragment { loadNearbyMapData(); binding = FragmentExploreBinding.inflate(inflater, container, false); - viewPagerAdapter = new ViewPagerAdapter(getChildFragmentManager()); + viewPagerAdapter = new ViewPagerAdapter(getChildFragmentManager(), + FragmentPagerAdapter.BEHAVIOR_RESUME_ONLY_CURRENT_FRAGMENT); + binding.viewPager.setAdapter(viewPagerAdapter); binding.viewPager.setId(R.id.viewPager); binding.tabLayout.setupWithViewPager(binding.viewPager); diff --git a/app/src/main/java/fr/free/nrw/commons/explore/map/ExploreMapFragment.java b/app/src/main/java/fr/free/nrw/commons/explore/map/ExploreMapFragment.java index a222a98ac..feebd20c6 100644 --- a/app/src/main/java/fr/free/nrw/commons/explore/map/ExploreMapFragment.java +++ b/app/src/main/java/fr/free/nrw/commons/explore/map/ExploreMapFragment.java @@ -221,7 +221,6 @@ public class ExploreMapFragment extends CommonsDaggerSupportFragment binding.mapView.getController().setZoom(ZOOM_LEVEL); } - performMapReadyActions(); binding.mapView.getOverlays().add(new MapEventsOverlay(new MapEventsReceiver() { @Override @@ -341,7 +340,12 @@ public class ExploreMapFragment extends CommonsDaggerSupportFragment !locationPermissionsHelper.checkLocationPermission(getActivity())) { isPermissionDenied = true; } - lastKnownLocation = MapUtils.getDefaultLatLng(); + + lastKnownLocation = getLastLocation(); + + if (lastKnownLocation == null) { + lastKnownLocation = MapUtils.getDefaultLatLng(); + } // if we came from 'Show in Explore' in Nearby, load Nearby map center and zoom if (isCameFromNearbyMap()) { @@ -974,9 +978,6 @@ public class ExploreMapFragment extends CommonsDaggerSupportFragment -0.07483536015053005, 1f); } } - if (!isCameFromNearbyMap()) { - moveCameraToPosition(new GeoPoint(latLnge.getLatitude(), latLnge.getLongitude())); - } return latLnge; } diff --git a/app/src/main/java/fr/free/nrw/commons/media/MediaInterface.kt b/app/src/main/java/fr/free/nrw/commons/media/MediaInterface.kt index ef0ef1f9c..643374e54 100644 --- a/app/src/main/java/fr/free/nrw/commons/media/MediaInterface.kt +++ b/app/src/main/java/fr/free/nrw/commons/media/MediaInterface.kt @@ -186,13 +186,25 @@ interface MediaInterface { ): Single companion object { + /** + * Retrieved thumbnail height will be about this tall, but must be at least this height. + * A larger number means higher thumbnail resolution but more network usage. + */ + const val THUMB_HEIGHT_PX = 450 + const val MEDIA_PARAMS = - "&prop=imageinfo|coordinates&iiprop=url|extmetadata|user&&iiurlwidth=640&iiextmetadatafilter=DateTime|Categories|GPSLatitude|GPSLongitude|ImageDescription|DateTimeOriginal|Artist|LicenseShortName|LicenseUrl" + "&prop=imageinfo|coordinates&iiprop=url|extmetadata|user&&iiurlheight=" + + THUMB_HEIGHT_PX + + "&iiextmetadatafilter=DateTime|Categories|GPSLatitude|GPSLongitude|" + + "ImageDescription|DateTimeOriginal|Artist|LicenseShortName|LicenseUrl" /** * fetches category detail(title, hidden) for each category along with File information */ const val MEDIA_PARAMS_WITH_CATEGORY_DETAILS = - "&clprop=hidden&prop=categories|imageinfo&iiprop=url|extmetadata|user&&iiurlwidth=640&iiextmetadatafilter=DateTime|GPSLatitude|GPSLongitude|ImageDescription|DateTimeOriginal|Artist|LicenseShortName|LicenseUrl" + "&clprop=hidden&prop=categories|imageinfo&iiprop=url|extmetadata|user&&iiurlheight=" + + THUMB_HEIGHT_PX + + "&iiextmetadatafilter=DateTime|GPSLatitude|GPSLongitude|ImageDescription|" + + "DateTimeOriginal|Artist|LicenseShortName|LicenseUrl" } } diff --git a/app/src/main/java/fr/free/nrw/commons/wikidata/model/gallery/ImageInfo.kt b/app/src/main/java/fr/free/nrw/commons/wikidata/model/gallery/ImageInfo.kt index 492e2e1f8..7645e438d 100644 --- a/app/src/main/java/fr/free/nrw/commons/wikidata/model/gallery/ImageInfo.kt +++ b/app/src/main/java/fr/free/nrw/commons/wikidata/model/gallery/ImageInfo.kt @@ -72,7 +72,6 @@ open class ImageInfo : Serializable { } fun getThumbUrl(): String { - updateThumbUrl() return thumbUrl ?: "" }