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

POINTS import: Invalid vertex attribute format+dimension #45

Closed
camnewnham opened this issue May 3, 2022 · 11 comments · Fixed by #46
Closed

POINTS import: Invalid vertex attribute format+dimension #45

camnewnham opened this issue May 3, 2022 · 11 comments · Fixed by #46

Comments

@camnewnham
Copy link
Contributor

This error occurs when using some sample files from the draco repository, such as https://github.com/google/draco/blob/master/testdata/pc_kd_color.drc

Full error text:

ArgumentException: Invalid vertex attribute format+dimension value (UNorm8 x 3, data size must be multiple of 4)
UnityEngine.Mesh+MeshData.SetVertexBufferParams (System.Int32 vertexCount, UnityEngine.Rendering.VertexAttributeDescriptor[] attributes) (at <07c89f7520694139991332d3cf930d48>:0)
Draco.DracoNative.CreateMesh (System.Boolean& calculateNormals, System.Boolean requireNormals, System.Boolean requireTangents, System.Int32 weightsAttributeId, System.Int32 jointsAttributeId, System.Boolean forceUnityLayout) (at Packages/com.atteneder.draco@4041c676d1/Runtime/Scripts/DracoNative.cs:540)

At a glance, it seems that Unity requires multiples of 4 for vertex attributes, but (presumably) these draco encoded attributes are 3xUInt8 (R,G,B instead of R,G,B,A).

What's the best way to go about converting this into a format Unity is friendly with?

@atteneder
Copy link
Owner

It's probably best to fill in alpha values of 255 before passing the vertex buffer to Unity. Question is at what stage (in the C++ wrapper or in managed C#).

@camnewnham
Copy link
Contributor Author

camnewnham commented May 4, 2022

Here's a brief test of this in these PRs (for discussion purposes)

This works by:
DracoUnity - determines whether padding should be applied, using a PaddedColorAttributeMap. Passes the padded number of components to native.
Draco - has an additional parameter to the GetAttributeData method which specifies an override for the stride of num_components.

At the moment, I'm not infilling with 255 as this would require a performance hit in C# or a bit more mess in C++. Most unlit shaders don't support alpha...

This fixes:
https://github.com/google/draco/blob/master/testdata/pc_kd_color.drc
https://github.com/google/draco/blob/master/testdata/point_cloud_no_qp.drc


There are unrelated issues with these:
https://github.com/google/draco/blob/master/testdata/cube_pc.drc
https://github.com/google/draco/blob/master/testdata/pc_color.drc

These fail at (*decoder)->DecodePointCloudFromBuffer(*buffer), and I am not sure why...

@camnewnham
Copy link
Contributor Author

Ah, looking at the 'unrelated issues' above, these are failing because DRACO_BACKWARDS_COMPATIBILITY_SUPPORTED is not enabled; so I don't see this as a particular issue, though it's worth noting as they won't be able to be included in the unit tests.

Context is point_cloud_decoder.cc

  // Check for version compatibility.
#ifdef DRACO_BACKWARDS_COMPATIBILITY_SUPPORTED
  ...
#else
  if (version_major_ != max_supported_major_version) {
    return Status(Status::UNKNOWN_VERSION, "Unsupported major version."); // Hits this line
  }
...
#endif

@atteneder
Copy link
Owner

@camnewnham
Yeah, not supporting older Draco files was a deliberate design decision (to achieve a smaller binary).
Devs can recompile the libs with this flag on, if they absolutely need to load older files.

@AR-Vsx
Copy link

AR-Vsx commented May 9, 2022

@camnewnham
When using your fork in Unity, my captured point cloud looks like this :
Screenshot 2022-05-09 123228
I'm creating the point cloud in a C++ Application like this:

builder.Start(capturedPC->count());
int pos_att_id = builder.AddAttribute(draco::GeometryAttribute::POSITION, 3,draco::DT_FLOAT32);
int color_att_id = builder.AddAttribute(draco::GeometryAttribute::COLOR, 3, draco::DT_UINT8);

for (draco::PointIndex i(0); i < capturedPC->count(); i++) 
{            
    builder.SetAttributeValueForPoint(pos_att_id, i, &points[i.value()]);
    builder.SetAttributeValueForPoint(color_att_id, i, &points[i.value()].r);
}

std::unique_ptr<draco::PointCloud> pc = builder.Finalize(true);

draco::Encoder e;
draco::EncoderBuffer eBuff;


e.EncodePointCloudToBuffer(*pc, &eBuff);
draco::WriteBufferToFile(ebuff.data(), ebuff.size(), pcEncFilename)

The points struct looks like this:

struct cwipc_point {
    float x;		/**< coordinate */
    float y;		/**< coordinate */
    float z;		/**< coordinate */
    uint8_t r;		/**< color */
    uint8_t g;		/**< color */
    uint8_t b;		/**< color */
    uint8_t tile;	/**< tile number (can also be interpreted as mask of contributing cameras) */
};

I'm also exporting the captured pointcloud as a ply file, which I encode with draco_encoder.exe -point_cloud and when I decode this file in Unity it looks fine. And when I decode the created drc file with draco_decoder.exe it also looks fine.
To render the color of the point cloud in Unity I'm using https://github.com/keijiro/Pcx

@camnewnham
Copy link
Contributor Author

camnewnham commented May 9, 2022

When using your fork in Unity

@AR-Vsx just to confirm, did you build the matching native Draco library? The native dlls are not included in this PR (for security only those built by the CI are included). atteneder/draco#6

Please let me know if that resolves your issue

@AR-Vsx
Copy link

AR-Vsx commented May 9, 2022

I have built dracodec_unity.dll from https://github.com/camnewnham/draco. Is that correct ? If it is, it didn't fix the issue.

@atteneder
Copy link
Owner

atteneder commented May 9, 2022

I have built dracodec_unity.dll from https://github.com/camnewnham/draco. Is that correct ?

Maybe. The PR references branch color-pad. Have you used this exact branch?

@AR-Vsx
Copy link

AR-Vsx commented May 10, 2022

No, I used the main branch. I switched branches, built the dll and it works fine now, Thank you and @camnewnham thank you too.

@atteneder
Copy link
Owner

Undoing unjust GH auto-close

@atteneder atteneder reopened this Jan 25, 2023
@atteneder
Copy link
Owner

If I'm not mistaken this was fixed with binaries 1.1.0 in DracoUnity 3.4.0 and 4.1.0 respectively.

atteneder added a commit to Unity-Technologies/DracoUnity that referenced this issue Sep 9, 2024
* chore: Removed obsolete funding info

* chore: Removing backstage info.
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 a pull request may close this issue.

3 participants