vagran / dxf-viewer

DXF 2D viewer written in JavaScript
Mozilla Public License 2.0
290 stars 86 forks source link

Boundingsphere radius is NaN when initializing raycaster. #55

Open dotoritos-kim opened 1 year ago

dotoritos-kim commented 1 year ago

error error2

It seems to be a problem that does not include the z position in the matrix. Considering the difficulty of development in the next release, I think we need a logic to put fake Z coordinate values.

vagran commented 1 year ago

What is the reproduction scenario for this issue? Currently there is no Z coordinates for rendering primitives, since this is huge memory wasting for planar designs.

dotoritos-kim commented 1 year ago

What is the reproduction scenario for this issue? Currently there is no Z coordinates for rendering primitives, since this is huge memory wasting for planar designs.

If you create a Raycaster object and ray tracing the object, the above image error occurs.

vagran commented 1 year ago

Yes, this will not be that simple with the current implementation. You can try to add Z coordinates in your fork. However, I doubt that approach used in three.js for Raycaster will have good performance with huge DXFs.

dotoritos-kim commented 1 year ago

Yes, this will not be that simple with the current implementation. You can try to add Z coordinates in your fork. However, I doubt that approach used in three.js for Raycaster will have good performance with huge DXFs.

logic is huge, so I failed to add the Z-axis as I thought. Could you please briefly tell me how to do it?

vagran commented 1 year ago

Probably some easiest "quick and dirty" way is modifying attributes arrays (instead of digging into batch construction code): https://github.com/vagran/dxf-viewer/blob/151b7def2bb3f5879d8e989a41367b3e69b39485/src/DxfViewer.js#L718 https://github.com/vagran/dxf-viewer/blob/151b7def2bb3f5879d8e989a41367b3e69b39485/src/DxfViewer.js#L735 These arrays initially contain X/Y coordinates as [X1,Y1,X2,Y2,...], should be modified to [X1, Y1, 0, X2, Y2, 0,...].

vertices: new three.BufferAttribute(verticesArray, 2), <<< should be changed to 3

Shader code probably should be changed as well: https://github.com/vagran/dxf-viewer/blob/151b7def2bb3f5879d8e989a41367b3e69b39485/src/DxfViewer.js#L586

in vec3 position;
...
vec4 pos = vec4(position, 1.0);

May be something else I missed. However, I doubt you will get what you want after that, each Three.js object is a whole batch, which consists from primitives from unrelated DXF entities, grouped by common material.

dotoritos-kim commented 1 year ago

image It seems to work better than predicted. The number of errors has been reduced from 62 to 38.

KaldrArt commented 1 year ago

What is the reproduction scenario for this issue? Currently there is no Z coordinates for rendering primitives, since this is huge memory wasting for planar designs.

yes,most memory wasting is due to TTF font entities. In my case, in order to catch mouse events by raycaster, I've added Z to my vertices, it costs lots of memory when there's lots of texts in the dxf file. And it wastes more time.

I think perhaps later we need to render shp fonts instead. Shp fonts have only lines instead of triangles which will save huge memory and rendering time.

KaldrArt commented 1 year ago

Yes, this will not be that simple with the current implementation. You can try to add Z coordinates in your fork. However, I doubt that approach used in three.js for Raycaster will have good performance with huge DXFs.

No performance problems on even low-profile integrated graphics, I've added some complicated animations in my case, for example, drawing and moving rectangle annotions, a small square that can be magnetically attached to the edge of the line and the intersection of lines.

Add an animate function in DxfViewer, when triggered some plugin, for example raycaster events, change some variable like somePluginEnabled , and call Animate()

    Animate() {
        if (this.HasRenderer()) {
            this.Render()
        }

        if(somePluginEnabled) requestAnimationFrame(this.Animate.bind(this))
    }

Because of your amazing work, three.js is perfectly combined with dxf-viewer.

vagran commented 1 year ago

I think perhaps later we need to render shp fonts instead. Shp fonts have only lines instead of triangles which will save huge memory and rendering time.

I had thoughts to migrate to text rendering based on signed distance field textures, which AFAIK is state-of-the-art, and combines advantages from both vector and raster worlds. Probably I should plan it as one more big feature in the next major release.

dotoritos-kim commented 1 year ago
    `function CreateObject(vertices, indices) {
        const geometry = instanceBatch
            ? new three.InstancedBufferGeometry()
            : new three.BufferGeometry();
        geometry.setAttribute('position', vertices);
        instanceBatch?._SetInstanceTransformAttribute(geometry);

        if (indices) {
            geometry.setIndex(indices);
        }

        const tmpVertices = geometry.attributes.position.array;
        const tmpPositions = new Float32Array(
            tmpVertices.length + tmpVertices.length / 2,
        );
        const tmpGeometry = instanceBatch
            ? new three.InstancedBufferGeometry()
            : new three.BufferGeometry();
        tmpGeometry.setAttribute(
            'position',
            new three.BufferAttribute(tmpPositions, 3),
        );

        let verticesIndex = 0;
        for (let posIdx = 0; posIdx < tmpVertices.length; posIdx += 2) {
            tmpGeometry.attributes.position.setXYZ(
                verticesIndex,
                tmpVertices[posIdx],
                tmpVertices[posIdx + 1],
                0,
            );

            verticesIndex++;
        }

        geometry.setAttribute('position', tmpGeometry.attributes.position);

        const obj = new objConstructor(geometry, material);
        obj.frustumCulled = false;
        obj.matrixAutoUpdate = false;
        return obj;
    }

    if (this.chunks) {
        for (const chunk of this.chunks) {
            yield CreateObject(chunk.vertices, chunk.indices);
        }
    } else {
        yield CreateObject(this.vertices);
    }`

If this code is added to DxfViewer.js CreateObject, it is possible to render at high speed without problems, and it is confirmed that the above errors do not appear. Try adding this code to DxfViewer or local fork

vagran commented 1 year ago

If this code is added to DxfViewer.js CreateObject, it is possible to render at high speed without problems, and it is confirmed that the above errors do not appear. Try adding this code to DxfViewer or local fork

This should be done in a fork, since it is a workaround which has no value for the library at the moment.