itowns
itowns copied to clipboard
Update outdated documentation
Following the recent changes, could be a good idea to read the doc and update it when necessary.
I stumbled across this:
which should be edit, using this
foo.addEventListener(
itowns.C3DTILES_LAYER_EVENTS.ON_TILE_CONTENT_LOADED,
updatePointCloudSize,
);
This is just an example of potential update. This issue can be used to list the edits that must be done.
Yes this documentation is outdated, I introduced this change with this PR and was not aware of this documentation. It should be updated with something as you propose except updatePointCloudSize
signature should change from
updatePointCloudSize(tileContent);
to
updatePointCloudSize({ tileContent });
I am wondering if the aim of this function is right though since there is an attribute material
in C3DTilesLayer
which is overriding points material see, meaning the user could write something like:
layer.material.size = 3;
to achieve the same thing. But this behavior of C3DTilesLayer
is not documented..
WDYT ?
Thanks for the feedback @valentinMachado !
If I'm not mistaking, layer.material is not defined and when the material is created here, it isn't assigned to layer.material.
It means that we should add this layer.material = material
but in ReferencingLayerProperties, we do the assignment the other way around : material.layer = layer
, this could result in a cyclic reference (not sure if it's really an issue).
Or, since we do not have access to the current material (from my understanding), we could directly give a new material:
function updatePointCloudSize({tileContent}) {
this.material = new itowns.PointsMaterial({
size:10,
mode: this.pntsMode,
shape: this.pntsShape,
classification: this.classification,
sizeMode: this.pntsSizeMode,
minAttenuatedSize: this.pntsMinAttenuatedSize,
maxAttenuatedSize: this.pntsMaxAttenuatedSize
});
}
Which goes against the purpose of the Style definition that we should use. This is a topic that should be more discussed, maybe in a related issue :thinking: Either way, your suggestion is relevant.