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

draco support requires deduplication support in draco #1671

Closed
svenevs opened this issue Oct 29, 2021 · 3 comments
Closed

draco support requires deduplication support in draco #1671

svenevs opened this issue Oct 29, 2021 · 3 comments

Comments

@svenevs
Copy link

svenevs commented Oct 29, 2021

Description of Issue

The draco plugin uses these deduplication methods:

https://github.com/PixarAnimationStudios/USD/blob/dc710925675f7f58f7a37f6c18d046a687b09271/pxr/usd/plugin/usdDraco/exportTranslator.cpp#L369-L373

The user needs to have configured their draco build with this enabled (e.g., cmake ..... -DDRACO_DECODER_ATTRIBUTE_DEDUPLICATION=ON), it's OFF by default in their build.

This could either be documented in BUILDING.md, or alternatively you can (after finding the draco include dirs) search the file include/draco/draco_features.h for

  • #define DRACO_ATTRIBUTE_INDICES_DEDUPLICATION_SUPPORTED
  • #define DRACO_ATTRIBUTE_VALUES_DEDUPLICATION_SUPPORTED

or alternatively, only call the methods guarded under the appropriate #ifdef (probably easiest, though it would result in no deduplication occurring). This is what they do:

https://github.com/google/draco/blob/5c0976b219b3f05b8935893ff38249b4db6088f5/src/draco/io/ply_decoder.cc#L81-L86

Steps to Reproduce

  1. Build and install draco with -DDRACO_DECODER_ATTRIBUTE_DEDUPLICATION=OFF.
  2. Compile USD with -DPXR_BUILD_DRACO_PLUGIN=ON and -DDRACO_ROOT=/path/to/draco_without_deduplication_support.

System Information (OS, Hardware)

Ubuntu 20.04

Package Versions

Build Flags

@jtran56
Copy link

jtran56 commented Oct 29, 2021

Filed as internal issue #USD-6993

@tomfinegan
Copy link
Contributor

Just a minor correction regarding the draco build configuration. I believe the rest of the issue is a legitimate concern. I'm not sure how critical given that the USD build defaults to use of the draco repository directly and builds from source, but if a user were to configure the build to use an installed draco library with the noted features disabled there would be build failures.

The user needs to have configured their draco build with this enabled (e.g., cmake ..... -
DDRACO_DECODER_ATTRIBUTE_DEDUPLICATION=ON), it's OFF by default in their build.

The above is incorrect, but I understand coming to this conclusion if looking only briefly at the CMake build and how the options are handled. By default Draco enables DRACO_ATTRIBUTE_INDICES_DEDUPLICATION_SUPPORTED and DRACO_ATTRIBUTE_VALUES_DEDUPLICATION_SUPPORTED .

The generated draco_features.h looks like this with defaults:

// GENERATED FILE -- DO NOT EDIT

#ifndef DRACO_FEATURES_H_
#define DRACO_FEATURES_H_

#define DRACO_POINT_CLOUD_COMPRESSION_SUPPORTED
#define DRACO_MESH_COMPRESSION_SUPPORTED
#define DRACO_NORMAL_ENCODING_SUPPORTED
#define DRACO_STANDARD_EDGEBREAKER_SUPPORTED
#define DRACO_PREDICTIVE_EDGEBREAKER_SUPPORTED
#define DRACO_BACKWARDS_COMPATIBILITY_SUPPORTED
#define DRACO_ATTRIBUTE_INDICES_DEDUPLICATION_SUPPORTED
#define DRACO_ATTRIBUTE_VALUES_DEDUPLICATION_SUPPORTED

#endif  // DRACO_FEATURES_H_

@sunyab
Copy link
Contributor

sunyab commented Jan 26, 2022

I've applied the fix to avoid calling the deduplication functions if the feature macros are not defined and it'll be part of the next push to dev.

However, I wanted to note that I wasn't able to get a Draco build with deduplication disabled, even after I specified -DDRACO_DECODER_ATTRIBUTE_DEDUPLICATION=OFF. Not sure if I was doing something wrong or if it's something in the Draco build itself.

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

No branches or pull requests

4 participants