iTowns / itowns

A Three.js-based framework written in Javascript/WebGL for visualizing 3D geospatial data
http://www.itowns-project.org
Other
1.08k stars 293 forks source link

Fix `View.pickObjectAt()` #2231

Closed mathieuLivebardon closed 6 months ago

mathieuLivebardon commented 8 months ago

Description

pickObjectAt returns an empty array if parameter where is an empty array.

Motivation and Context

where is a parameter of pickObjectAt to specify the layers to perform a pick. The problem: If where is an empty array, the function picks on all layers by default.

const intersects = view.pickObjectsAt(
  event,
  0,
  view.getLayers().filter((el) => el.isC3DTilesLayer)
);

Here, if the application had dynamic layers¹ and no C3DTilesLayers in the View, intersects should be an empty array.

¹ remove/add layers at runtime

ketourneau commented 7 months ago

I think your PR can cause some trouble for people who use the pickObjectAt and want method to use all layers if where condition is an empty array.

In your case, why not simply check empty array before call pickObjectsAt ?

const layers = view.getLayers().filter((el) => el.isC3DTilesLayer);
let intersects = [];
if (layers.length > 0) {
  intersects = view.pickObjectsAt(event, 0, ayers);
}
jailln commented 7 months ago

I agree with @ketourneau, and how it works currently seems more intuitive to me. Would the solution proposed by @ketourneau work in your context?

mathieuLivebardon commented 6 months ago

Fine, I'll close this PR and use @ketourneau's solution. Thanks for your answers.