Skip to content

Commit 0120207

Browse files
Jason-WhitmoreRitikaPahwa4444nicolas-raoul
authored
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 <[email protected]> Co-authored-by: Nicolas Raoul <[email protected]>
1 parent 3f2077a commit 0120207

File tree

5 files changed

+35
-9
lines changed

5 files changed

+35
-9
lines changed

app/src/main/java/fr/free/nrw/commons/ViewPagerAdapter.java

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,17 @@ public ViewPagerAdapter(FragmentManager manager) {
1818
super(manager);
1919
}
2020

21+
/**
22+
* Constructs a ViewPagerAdapter with a specified Fragment Manager and Fragment resume behavior.
23+
*
24+
* @param manager The FragmentManager
25+
* @param behavior An integer which represents the behavior of non visible fragments. See
26+
* FragmentPagerAdapter.java for options.
27+
*/
28+
public ViewPagerAdapter(FragmentManager manager, int behavior) {
29+
super(manager, behavior);
30+
}
31+
2132
/**
2233
* This method returns the fragment of the viewpager at a particular position
2334
* @param position

app/src/main/java/fr/free/nrw/commons/explore/ExploreFragment.java

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
import androidx.annotation.NonNull;
1313
import androidx.annotation.Nullable;
1414
import androidx.fragment.app.Fragment;
15+
import androidx.fragment.app.FragmentPagerAdapter;
1516
import androidx.viewpager.widget.ViewPager.OnPageChangeListener;
1617
import fr.free.nrw.commons.R;
1718
import fr.free.nrw.commons.ViewPagerAdapter;
@@ -69,7 +70,9 @@ public View onCreateView(LayoutInflater inflater, @Nullable ViewGroup container,
6970
loadNearbyMapData();
7071
binding = FragmentExploreBinding.inflate(inflater, container, false);
7172

72-
viewPagerAdapter = new ViewPagerAdapter(getChildFragmentManager());
73+
viewPagerAdapter = new ViewPagerAdapter(getChildFragmentManager(),
74+
FragmentPagerAdapter.BEHAVIOR_RESUME_ONLY_CURRENT_FRAGMENT);
75+
7376
binding.viewPager.setAdapter(viewPagerAdapter);
7477
binding.viewPager.setId(R.id.viewPager);
7578
binding.tabLayout.setupWithViewPager(binding.viewPager);

app/src/main/java/fr/free/nrw/commons/explore/map/ExploreMapFragment.java

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -221,7 +221,6 @@ public void onViewCreated(@NonNull final View view, @Nullable final Bundle saved
221221
binding.mapView.getController().setZoom(ZOOM_LEVEL);
222222
}
223223

224-
performMapReadyActions();
225224

226225
binding.mapView.getOverlays().add(new MapEventsOverlay(new MapEventsReceiver() {
227226
@Override
@@ -341,7 +340,12 @@ private void performMapReadyActions() {
341340
!locationPermissionsHelper.checkLocationPermission(getActivity())) {
342341
isPermissionDenied = true;
343342
}
344-
lastKnownLocation = MapUtils.getDefaultLatLng();
343+
344+
lastKnownLocation = getLastLocation();
345+
346+
if (lastKnownLocation == null) {
347+
lastKnownLocation = MapUtils.getDefaultLatLng();
348+
}
345349

346350
// if we came from 'Show in Explore' in Nearby, load Nearby map center and zoom
347351
if (isCameFromNearbyMap()) {
@@ -974,9 +978,6 @@ public fr.free.nrw.commons.location.LatLng getMapCenter() {
974978
-0.07483536015053005, 1f);
975979
}
976980
}
977-
if (!isCameFromNearbyMap()) {
978-
moveCameraToPosition(new GeoPoint(latLnge.getLatitude(), latLnge.getLongitude()));
979-
}
980981
return latLnge;
981982
}
982983

app/src/main/java/fr/free/nrw/commons/media/MediaInterface.kt

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -186,13 +186,25 @@ interface MediaInterface {
186186
): Single<MwQueryResponse>
187187

188188
companion object {
189+
/**
190+
* Retrieved thumbnail height will be about this tall, but must be at least this height.
191+
* A larger number means higher thumbnail resolution but more network usage.
192+
*/
193+
const val THUMB_HEIGHT_PX = 450
194+
189195
const val MEDIA_PARAMS =
190-
"&prop=imageinfo|coordinates&iiprop=url|extmetadata|user&&iiurlwidth=640&iiextmetadatafilter=DateTime|Categories|GPSLatitude|GPSLongitude|ImageDescription|DateTimeOriginal|Artist|LicenseShortName|LicenseUrl"
196+
"&prop=imageinfo|coordinates&iiprop=url|extmetadata|user&&iiurlheight=" +
197+
THUMB_HEIGHT_PX +
198+
"&iiextmetadatafilter=DateTime|Categories|GPSLatitude|GPSLongitude|" +
199+
"ImageDescription|DateTimeOriginal|Artist|LicenseShortName|LicenseUrl"
191200

192201
/**
193202
* fetches category detail(title, hidden) for each category along with File information
194203
*/
195204
const val MEDIA_PARAMS_WITH_CATEGORY_DETAILS =
196-
"&clprop=hidden&prop=categories|imageinfo&iiprop=url|extmetadata|user&&iiurlwidth=640&iiextmetadatafilter=DateTime|GPSLatitude|GPSLongitude|ImageDescription|DateTimeOriginal|Artist|LicenseShortName|LicenseUrl"
205+
"&clprop=hidden&prop=categories|imageinfo&iiprop=url|extmetadata|user&&iiurlheight=" +
206+
THUMB_HEIGHT_PX +
207+
"&iiextmetadatafilter=DateTime|GPSLatitude|GPSLongitude|ImageDescription|" +
208+
"DateTimeOriginal|Artist|LicenseShortName|LicenseUrl"
197209
}
198210
}

app/src/main/java/fr/free/nrw/commons/wikidata/model/gallery/ImageInfo.kt

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,6 @@ open class ImageInfo : Serializable {
7272
}
7373

7474
fun getThumbUrl(): String {
75-
updateThumbUrl()
7675
return thumbUrl ?: ""
7776
}
7877

0 commit comments

Comments
 (0)