Skip to content

Fix bounds for empty text nodes#1062

Merged
robertosfield merged 1 commit intoopenscenegraph:masterfrom
calumr:empty-text-nodes-bounds
Apr 14, 2021
Merged

Fix bounds for empty text nodes#1062
robertosfield merged 1 commit intoopenscenegraph:masterfrom
calumr:empty-text-nodes-bounds

Conversation

@calumr
Copy link
Copy Markdown
Contributor

@calumr calumr commented Apr 13, 2021

_lineCount can be 0 if the TextBase has no text, so we get underflow if the alignment is one of the *_BOTTOM_BASE_LINE. Empty text nodes seem to come up in some of the .dxf files our users are importing.

I've worked around the bug in our code for now by deleting the empty text nodes on load.

@robertosfield
Copy link
Copy Markdown
Collaborator

Could provide details of how to reproduce the problem?

I think it would be better to have a dedicated code at the start of the method, along the lines of:

 if (empty) { set fallback values; return; }

@calumr
Copy link
Copy Markdown
Contributor Author

calumr commented Apr 13, 2021

    osg::ref_ptr<osg::Node> root = osgDB::readRefNodeFile("c:\\dev\\emptyTextNodes.dxf");

    osg::ComputeBoundsVisitor cbv;
    root->accept(cbv);
    if (cbv.getBoundingBox().xMax() > 10000)
    {
        osg::notify(osg::NOTICE) << "too big: "<< cbv.getBoundingBox().xMax() << std::endl;
    }

Run that on the attached .dxf, it should print "too big: 7.50612e+09".

emptyTextNodes.zip

@calumr calumr force-pushed the empty-text-nodes-bounds branch from 994be8f to bc0d36a Compare April 13, 2021 13:18
@calumr
Copy link
Copy Markdown
Contributor Author

calumr commented Apr 13, 2021

I've updated the change to check if (_text.empty()) as you suggested.

@openscenegraph
Copy link
Copy Markdown
Owner

openscenegraph commented Apr 13, 2021 via email

_lineCount can be 0, so we get underflow if the alignment is one of
the *_BOTTOM_BASE_LINE.
@calumr calumr force-pushed the empty-text-nodes-bounds branch from bc0d36a to f84072c Compare April 14, 2021 10:04
@calumr
Copy link
Copy Markdown
Contributor Author

calumr commented Apr 14, 2021

All done. I had to move the _normal calculation up above the if, I hope this is OK. There's no rush on this, thanks for taking a look.

@robertosfield
Copy link
Copy Markdown
Collaborator

Looks good!

@robertosfield robertosfield merged commit 4dad4af into openscenegraph:master Apr 14, 2021
@robertosfield
Copy link
Copy Markdown
Collaborator

Also merged with 3.6 and MeshShaders branches.

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.

3 participants