Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implemented load and display nearby places (https://github.com/osmand… #21771

Merged
merged 6 commits into from
Jan 22, 2025

Conversation

Corwin-Kh
Copy link
Contributor

…app/OsmAnd-Issues/issues/2662)

@@ -278,6 +279,7 @@ public void onStop(@NonNull LifecycleOwner owner) {
localeHelper.checkPreferredLocale();
appInitializer.onCreateApplication();
osmandMap.getMapLayers().createLayers(osmandMap.getMapView());
NearbyPlacesHelper.INSTANCE.init(this);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Better move to onCreateApplication()

@@ -138,6 +142,21 @@ public Drawable getIcon(@DrawableRes int id, @ColorRes int colorId) {
return getDrawable(id, colorId);
}

@Nullable
public Drawable getRenderingIcon(Context ctx, String fileName, boolean nightMode) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

parameters annotations missing

@@ -748,4 +767,14 @@ public static void updateStatusBarColor(@NonNull MapActivity activity) {

AndroidUiHelper.setStatusBarContentColor(activity.getWindow().getDecorView(), nightModeForContent);
}

public Bitmap getScaledBitmap(@Nullable MapActivity activity, @DrawableRes int drawableId, float scale) {
Copy link
Member

@Chumva Chumva Jan 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

better use @UiContext Context instead of activity, if is not necessary

<ImageView
android:id="@+id/item_image"
android:layout_width="wrap_content"
android:layout_height="108dp"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Move to dimens for easier support

@@ -973,6 +973,7 @@ public void onDismiss(@NonNull DialogInterface dialog) {
fragmentManager.popBackStack();
}
}
app.getOsmandMap().getMapLayers().getNearbyPlacesLayer().customObjectsDelegate.setCustomMapObjects(null);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this place correct, won`t screen rotation break it? onBackButtonPressed()?

also we could try to use similar logic in NearbyPlacesHelper as in PoiFiltersHelper, without using directly NearbyPlacesLayer

@@ -60,6 +73,38 @@ private PicassoUtils(@NonNull OsmandApplication app) {
}
}

public OkHttpClient getUnsafeOkHttpClient() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks strange, is it needed only in debug builds?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it is a point of discussion if we need it or there is any more correct way to fix download image issue

return lastModifiedTime
}

fun createSmallPointBitmap(layer: NearbyPlacesLayer): Bitmap {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Move bitmaps creation from helper, for example to PointImageUtils

private final List<Target> imageLoadingTargets = new ArrayList<>();


public CustomMapObjects<NearbyPlacePoint> customObjectsDelegate = new OsmandMapLayer.CustomMapObjects<>();
Copy link
Member

@Chumva Chumva Jan 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need a separate collection, or we can use data from helper or load it from internet similar to OsmBugsLayer ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it is made to have more control of what is shown on the layer

float x = tileBox.getPixXFromLatLon(lat, lon);
float y = tileBox.getPixYFromLatLon(lat, lon);
if (intersects(boundIntersections, x, y, iconSize, iconSize) || nearbyPoint.imageBitmap == null) {
canvas.drawBitmap(NearbyPlacesHelper.INSTANCE.createSmallPointBitmap(this), x, y, new Paint());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is discouraged to spam objects creation during drawing, you could store new Paint() as class variable

Copy link
Member

@Chumva Chumva Jan 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, we draw many smallPointBitmaps, but they are the same ones, probably better to reuse bitmaps, or create some kind of cache, similar to one in PointImageUtils for opengl & legacy renderers

Comment on lines +72 to +73
prevMapRect!!, prevZoom!!,
prevLang!!, loadNearbyPlacesListener).execute()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Try to avoid using !!

if (bitmap != null) {
float x = tileBox.getPixXFromLatLon(point.getLatitude(), point.getLongitude());
float y = tileBox.getPixYFromLatLon(point.getLatitude(), point.getLongitude());
canvas.drawBitmap(cachedSmallIconBitmap, x, y, new Paint());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We do not draw preview images for V1?

Comment on lines +59 to +62
private int smallIconSize;
private int bigIconSize;
private int bigIconBorderSize;
private int smallIconBorderSize;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These variables could be moved to images creation in PointImageUtils

@@ -126,4 +138,55 @@ private PointImageInfo(@NonNull BackgroundType type, @ColorInt int color,
this.type = type;
}
}
public static Bitmap createSmallPointBitmap(NearbyPlacesLayer layer) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we could get all variables here without layer


import org.apache.commons.logging.Log;

public class NearbyPlacesFragment extends BaseOsmAndFragment {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need this only for toolbar? better create something similar to QuickSearchToolbarController

private NearbyPlacesAdapter adapter = null;
private NearbyPlacesAdapter adapter;
private ImageView explicitIndicator;
private boolean expanded;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could store expanded state in preference similar to CollapsableView

@Chumva
Copy link
Member

Chumva commented Jan 22, 2025

The first part could be merged for testing, but it needs further improvements & polishing

@Chumva Chumva merged commit e46c356 into master Jan 22, 2025
3 checks passed
@Chumva Chumva deleted the nearby_places_clear branch January 22, 2025 22:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants