search

iTowns / itowns

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

fix(entwine): change transparency settings #2327

Closed ftoromanoff closed 1 month ago

ftoromanoff commented 1 month ago

Fix for issue #2323.

This issue point out a bug with the rendering of point cloud where points should be rendered with differentes values of opacity.

airnez commented 1 month ago

Do you think it will fix this issue (#2323) ?

ftoromanoff commented 1 month ago

Do you think it will fix this issue (#2323) ?

Currently, I only have look up for a fix for PointCloudLayer (entwine Potree Layers). I will try to continue toward 3DTilesLayer

airnez commented 1 month ago

Do you think it will fix this issue (#2323) ?

Currently, I only have look up for a fix for PointCloudLayer (entwine Potree Layers). I will try to continue toward 3DTilesLayer

Understood. Thank you for your answer 👍

AnthonyGlt commented 1 month ago

Thank you @ftoromanoff for this MR ! Looks like it solves the transparency issue :+1: It also exposed the fact that the 3dtiles is a PointCloud (or contains points) with hasPnts. I like this idea. Looks good to me !

I have an interrogation, it's not really related to this MR but it's about PointsMaterial.js

https://github.com/iTowns/itowns/blob/0e822cb268ad702df52020e58a1029ac21472b03/src/Renderer/PointsMaterial.js#L49-L80

WHat's the point of having a DEFAULT object in the ClassificationScheme & in the DiscreteScheme ? Couldn't we put the DiscreteScheme directly in the ClassificationScheme ? We could access it with ClassificationScheme.DISCRETE

export const ClassificationScheme = {
    DEFAULT: {
        0: { visible: true, name: 'never classified', color: new THREE.Color(0.5,  0.5,  0.5), opacity: 1.0 },
        1: { visible: true, name: 'unclassified', color: new THREE.Color(0.5,  0.5,  0.5), opacity: 1.0 },
        2: { visible: true, name: 'ground', color: new THREE.Color(0.63, 0.32, 0.18), opacity: 1.0 },
        3: { visible: true, name: 'low vegetation', color: new THREE.Color(0.0,  1.0,  0.0), opacity: 1.0 },
        4: { visible: true, name: 'medium vegetation', color: new THREE.Color(0.0,  0.8,  0.0), opacity: 1.0 },
        5: { visible: true, name: 'high vegetation', color: new THREE.Color(0.0,  0.6,  0.0), opacity: 1.0 },
        6: { visible: true, name: 'building', color: new THREE.Color(1.0,  0.66, 0.0), opacity: 1.0 },
        7: { visible: true, name: 'low point(noise)', color: new THREE.Color(1.0,  0.0,  1.0), opacity: 1.0 },
        8: { visible: true, name: 'key-point', color: new THREE.Color(1.0,  0.0,  0.0), opacity: 1.0 },
        9: { visible: true, name: 'water', color: new THREE.Color(0.0,  0.0,  1.0), opacity: 1.0 },
        10: { visible: true, name: 'rail', color: new THREE.Color(0.8,  0.8,  1.0), opacity: 1.0 },
        11: { visible: true, name: 'road Surface', color: new THREE.Color(0.4,  0.4,  0.7), opacity: 1.0 },
        12: { visible: true, name: 'overlap', color: new THREE.Color(1.0,  1.0,  0.0), opacity: 1.0 },
        DEFAULT: { visible: true, name: 'default', color: new THREE.Color(0.3, 0.6, 0.6), opacity: 1.0 },
    },
 DISCRETE : {

        0: { visible: true, name: '0', color: new THREE.Color('rgb(67, 99, 216)'), opacity: 1.0 },
        1: { visible: true, name: '1', color: new THREE.Color('rgb(60, 180, 75);'), opacity: 1.0 },
        2: { visible: true, name: '2', color: new THREE.Color('rgb(255, 255, 25)'), opacity: 1.0 },
        3: { visible: true, name: '3', color: new THREE.Color('rgb(145, 30, 180)'), opacity: 1.0 },
        4: { visible: true, name: '4', color: new THREE.Color('rgb(245, 130, 49)'), opacity: 1.0 },
        5: { visible: true, name: '5', color: new THREE.Color('rgb(230, 25, 75)'), opacity: 1.0 },
        6: { visible: true, name: '6', color: new THREE.Color('rgb(66, 212, 244)'), opacity: 1.0 },
        7: { visible: true, name: '7', color: new THREE.Color('rgb(240, 50, 230)'), opacity: 1.0 },
        DEFAULT: { visible: true, name: 'default', color: white, opacity: 1.0 },
    },
};

WDYT ?

ftoromanoff commented 1 month ago

Thank you @ftoromanoff for this MR ! Looks like it solves the transparency issue 👍 It also exposed the fact that the 3dtiles is a PointCloud (or contains points) with hasPnts. I like this idea. Looks good to me !

I have an interrogation, it's not really related to this MR but it's about PointsMaterial.js

https://github.com/iTowns/itowns/blob/0e822cb268ad702df52020e58a1029ac21472b03/src/Renderer/PointsMaterial.js#L49-L80

WHat's the point of having a DEFAULT object in the ClassificationScheme & in the DiscreteScheme ? Couldn't we put the DiscreteScheme directly in the ClassificationScheme ? We could access it with ClassificationScheme.DISCRETE

export const ClassificationScheme = {
    DEFAULT: {
        0: { visible: true, name: 'never classified', color: new THREE.Color(0.5,  0.5,  0.5), opacity: 1.0 },
        1: { visible: true, name: 'unclassified', color: new THREE.Color(0.5,  0.5,  0.5), opacity: 1.0 },
        2: { visible: true, name: 'ground', color: new THREE.Color(0.63, 0.32, 0.18), opacity: 1.0 },
        3: { visible: true, name: 'low vegetation', color: new THREE.Color(0.0,  1.0,  0.0), opacity: 1.0 },
        4: { visible: true, name: 'medium vegetation', color: new THREE.Color(0.0,  0.8,  0.0), opacity: 1.0 },
        5: { visible: true, name: 'high vegetation', color: new THREE.Color(0.0,  0.6,  0.0), opacity: 1.0 },
        6: { visible: true, name: 'building', color: new THREE.Color(1.0,  0.66, 0.0), opacity: 1.0 },
        7: { visible: true, name: 'low point(noise)', color: new THREE.Color(1.0,  0.0,  1.0), opacity: 1.0 },
        8: { visible: true, name: 'key-point', color: new THREE.Color(1.0,  0.0,  0.0), opacity: 1.0 },
        9: { visible: true, name: 'water', color: new THREE.Color(0.0,  0.0,  1.0), opacity: 1.0 },
        10: { visible: true, name: 'rail', color: new THREE.Color(0.8,  0.8,  1.0), opacity: 1.0 },
        11: { visible: true, name: 'road Surface', color: new THREE.Color(0.4,  0.4,  0.7), opacity: 1.0 },
        12: { visible: true, name: 'overlap', color: new THREE.Color(1.0,  1.0,  0.0), opacity: 1.0 },
        DEFAULT: { visible: true, name: 'default', color: new THREE.Color(0.3, 0.6, 0.6), opacity: 1.0 },
    },
 DISCRETE : {

        0: { visible: true, name: '0', color: new THREE.Color('rgb(67, 99, 216)'), opacity: 1.0 },
        1: { visible: true, name: '1', color: new THREE.Color('rgb(60, 180, 75);'), opacity: 1.0 },
        2: { visible: true, name: '2', color: new THREE.Color('rgb(255, 255, 25)'), opacity: 1.0 },
        3: { visible: true, name: '3', color: new THREE.Color('rgb(145, 30, 180)'), opacity: 1.0 },
        4: { visible: true, name: '4', color: new THREE.Color('rgb(245, 130, 49)'), opacity: 1.0 },
        5: { visible: true, name: '5', color: new THREE.Color('rgb(230, 25, 75)'), opacity: 1.0 },
        6: { visible: true, name: '6', color: new THREE.Color('rgb(66, 212, 244)'), opacity: 1.0 },
        7: { visible: true, name: '7', color: new THREE.Color('rgb(240, 50, 230)'), opacity: 1.0 },
        DEFAULT: { visible: true, name: 'default', color: white, opacity: 1.0 },
    },
};

WDYT ?

The discrete cheme is indeed a scheme for point classification. But ClassificationScheme is for the 'classification' point classification. That's the reason why there is 2 differentes schemes, One for the classification using point classification and an other one for all the other (return number, type of return etc...). We can imagine that in a future (or a user) want to have different discrete scheme: one for the type of return and another one for the point source ID.

ftoromanoff commented 1 month ago

@AnthonyGlt Can you start a review, and validate it if you have no remarks on the code ? Thks