Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

destroy objects in reverse order of dependency #31

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ghost
Copy link

@ghost ghost commented Jan 20, 2022

No description provided.

@ponchio
Copy link
Contributor

ponchio commented Jan 20, 2022

Hi Gernot,
it is not clear to me what is the rationale behind this change, could you explain?

@ghost
Copy link
Author

ghost commented Jan 20, 2022

In some cases related to ply files, if the crt::MeshLoader object is destructed before the crt::Encoder object, an exception will be thrown. I haven't delved deeply into this, but the order of destruction seems to matter.

@ponchio
Copy link
Contributor

ponchio commented Jan 20, 2022

I had a look at the code and found no plausible reason on why the destruction order should matter: the MeshLoader data is copied into Encoder (quantized actually). I made a test intentionally calling the destroyer but couldn't replicate the issue.

I guess the culprit is a wrong memory access problem somewhere else in the code. If I could manage to replicate it, I would rather find and fix the real problem. Which compiler where you using?

@ghost
Copy link
Author

ghost commented Jan 20, 2022

Apple clang version 13.0.0 (clang-1300.0.29.30)

@Rabbid76 Rabbid76 deleted the destroy-objects-in-reverse-order-of-dependency branch January 21, 2022 15:34
@ponchio
Copy link
Contributor

ponchio commented Jan 21, 2022

Hi ghost,

i made some tests in apple with clang, but with no luck: I can't replicate the issue.
Could you provide the model (and command line in case of options), or better, if you can, run valgrind?

@Rabbid76
Copy link

This PR can be ignored an closed. The actual problem is PLY files with QUAD faces are not read correctly #32

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.

2 participants