UsdGeomPoints import support #106398
Labels
No Label
Interest
Alembic
Interest
Animation & Rigging
Interest
Asset System
Interest
Audio
Interest
Automated Testing
Interest
Blender Asset Bundle
Interest
BlendFile
Interest
Collada
Interest
Compatibility
Interest
Compositing
Interest
Core
Interest
Cycles
Interest
Dependency Graph
Interest
Development Management
Interest
EEVEE
Interest
Freestyle
Interest
Geometry Nodes
Interest
Grease Pencil
Interest
ID Management
Interest
Images & Movies
Interest
Import Export
Interest
Line Art
Interest
Masking
Interest
Metal
Interest
Modeling
Interest
Modifiers
Interest
Motion Tracking
Interest
Nodes & Physics
Interest
OpenGL
Interest
Overlay
Interest
Overrides
Interest
Performance
Interest
Physics
Interest
Pipeline, Assets & IO
Interest
Platforms, Builds & Tests
Interest
Python API
Interest
Render & Cycles
Interest
Render Pipeline
Interest
Sculpt, Paint & Texture
Interest
Text Editor
Interest
Translations
Interest
Triaging
Interest
Undo
Interest
USD
Interest
User Interface
Interest
UV Editing
Interest
VFX & Video
Interest
Video Sequencer
Interest
Viewport & EEVEE
Interest
Virtual Reality
Interest
Vulkan
Interest
Wayland
Interest
Workbench
Interest: X11
Legacy
Asset Browser Project
Legacy
Blender 2.8 Project
Legacy
Milestone 1: Basic, Local Asset Browser
Legacy
OpenGL Error
Meta
Good First Issue
Meta
Papercut
Meta
Retrospective
Meta
Security
Module
Animation & Rigging
Module
Core
Module
Development Management
Module
Grease Pencil
Module
Modeling
Module
Nodes & Physics
Module
Pipeline, Assets & IO
Module
Platforms, Builds & Tests
Module
Python API
Module
Render & Cycles
Module
Sculpt, Paint & Texture
Module
Triaging
Module
User Interface
Module
VFX & Video
Module
Viewport & EEVEE
Platform
FreeBSD
Platform
Linux
Platform
macOS
Platform
Windows
Severity
High
Severity
Low
Severity
Normal
Severity
Unbreak Now!
Status
Archived
Status
Confirmed
Status
Duplicate
Status
Needs Info from Developers
Status
Needs Information from User
Status
Needs Triage
Status
Resolved
Type
Bug
Type
Design
Type
Known Issue
Type
Patch
Type
Report
Type
To Do
No Milestone
No project
No Assignees
6 Participants
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: blender/blender#106398
Loading…
Reference in New Issue
Block a user
No description provided.
Delete Branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
This issue aims to describe a task that will support the import of colorized point clouds to blender via the USD type UsdGeomPoints. It's also the goal of this design task to support the
.timeSampled
extension of both thepoints3f[]
andcolor3f[]
attributes in a UsdGeomPoints prim. This contribution would also aim to directly create blender point types, instead of making vertices that need to be converted via a geometry nodeMesh to points
kind of step.Related tasks that this will be separate from
This feature would support many users over the last few years who've posted about not being able to bring in colored point clouds. Some of these posts go back to pre blender 3, but this feature is still something that more recent users have needed also. Some issues relate to a different format .ply which is common when representing point clouds, but suffers when it comes to representing sequences of point clouds 1 2. It's for this reason that USD is such an appealing format with it's explicit
.timeSampled
attribute options.I'm looking forward to spending time on this, but will probably need a certain degree of support from other members of the USD format team. Thanks for all your incredible work!
To me it looks like this boils down to two features in the end:
UsdGeomPoints
should end up as a BlenderPointCloud
data-block.Do you think it's reasonable to break up the work this way?
@HooglyBoogly thanks for the the information. It sounds like exactly the approach I should be taking. if the UsdGeomPoint type is converted into a blender point cloud data block that would save the user from doing any manual conversions on their own. And I also agree that support for generic custom attribute information attached to each point would be a real game changer!
my plan is to first review how the mesh reader already written is parsing USD files and creating blender specific information. I've been hoped to start writing some code of my own now that I have completed my first blender compilation build. I believe in the USD mesh reader there is already some code (provided by @makowalski ? ) demonstrates how to handle generic attribute information which all study up on.
@devin-bayly There is a work in progress adding support for generic attributes for meshes here:
#105347
I would suggest waiting until this patch is done before you start this task. That task will hopefully be committed soon. @CharlesWardlaw implemented that pull request and could answer questions about that feature. I would also suggest that we discuss your proposed design and implementation (most likely a new
USDPointsReader
class, or something similar) before you begin.Sounds great @makowalski. I'm new to blender contribution so I'll take the time to explore topics related to data-blocks and some of the other existing code written for parsing USD files.
Ok I look forward to chatting with Charles when the time comes. Is it best to use the #blender-chat USD channel for that?
Sure I like this plan. I don't have too many existing ideas to propose but after I have a chance to look through the patch and the blender data-block C code I hope I'll have more on this to propose.
I was just looking at the issue conversation over at #105347 and I see a merged icon at the bottom, does this mean that the patch supporting generic mesh attributes is done?
I'm having a good time going through the orielly book about blender development. I'm focusing on the section about the RNA and Data api since I imagine that's what I'll be needing to interface with after reading the geom usd files. If there's other relevant material I should be studying let me know. Thanks so much!
@CharlesWardlaw for visibility.
I believe that patch covers color conversion only, but it's a good example of working with attributes.
If you haven't done so already, I would take a look at how the various USD and Alembic "reader" classes are implemented. BTW, the Alembic importer implements a mesh-based points reader that might be of interest:
https://projects.blender.org/blender/blender/src/branch/main/source/blender/io/alembic/intern/abc_reader_points.h
https://projects.blender.org/blender/blender/src/branch/main/source/blender/io/alembic/intern/abc_reader_points.cc
Yes, please @kattkieru me in the chat and I'll appear, as if Final Fantasy summoned. 😆
Yes, it was merged. It's a good example of how to handle the colors, and how USD and Blender each deal with them differently. If you look at the import and export sides, you'll see some conversion going on. There's also the interesting wrinkle that USD can specify a constant single color as applying to a whole prim's points, which on the Blender side needs to be expanded into the full generic attribute array.
For this particular task I think you wouldn't be touching DNA/RNA (and honestly if you can avoid doing so I recommend it). If the APIs for generic attributes are already available for Point Cloud objects, then all you need is to add new classes to handle Point Cloud import and export. If you look at usd_hierarchy_iterator.cc, you'll see a spot where an instance of a writer class could be returned (right now
OB_POINTCLOUD
type objects are set to return nullptr, meaning they are unhandled). The import side would be handled in usd_reader_stage.cc, looking at methodscreate_reader
andcreate_reader_if_allowed
.If you can get it to the point where you're able to comfortably round-trip even just static point positions, then you'll be about half of the way there and the rest of the bits should fall into place. There's already code for adding Mesh Cache modifiers for animated points, so the "hardest" part will be making sure that the data translates correctly and looks the same on both sides.
The only other thing I'd add is that once you've got it working and you like the numbers you're seeing, I highly recommend also adding to the Python test suite. (I'd avoid doing the tests in C++ now that we have the Python API unless you really, really enjoy pain.) That said, I think following Hans' suggestion above is good, meaning you'd have three patches:
Ping me if you have any questions, and thanks for volunteering to help!
@CharlesWardlaw This is wonderful! Thanks for saving me the time from trying to fully understand the RNA DNA side. Perhaps one day, but not for this contribution. It's funny, with each week that passes I come up with other projects that this will really help out =D. I'll make sure to reach out to you over the chat when I get stuck.
Good to know that #105347 is about some but not all the same things.
Great reading suggestions, and I'll step through an execution with an existing USD to see how their contents are used.
Yea, I hope to figure out the animated stuff after. It is really appealing to have that one day, but static point clouds is enough to work out first. Is the Mesh Cache Modifier how other USD
time.sampled
mesh properties change from one frame to another in Blender?I like that breakdown. The first patch would then require the class that @makowalski mentioned a number of posts back. #106398 (comment)
By the way, the mesh cache modifier won't work for the point cloud object type. That part will probably have to wait for a geometry nodes solution. That's a fine limitation for now though IMO.
ah good to know. Would that mean that it is going to be a significant undertaking to play back point clouds that change over time? Or just via that specific mesh cache modifier?
Right, I suppose that might not be possible for now, using the point cloud object type.
@makowalski With regards to understanding the alembic reader, do you have an example alembic file that I can use to explore this reader with? I'm having trouble getting pyalembic built to make one with, and I'm concerned that exporting a particle system from blender might give me one that is too complex ti understand on my own.
@HooglyBoogly oh shoot, ok.
A big part of the contribution motivation for me that I'm supporting a researcher who has timeseries lidar data, and I'm hoping to enable her to be self sufficient in blender for her rendering needs. For that should I rely on an alembic workflow instead? Perhaps we can continue this topic of conversation in the blender chat since this issue might not be the best place for a length discussion of my options.
@devin-bayly I'm not an expert on exporting to the alembic format, but I'm attaching two trivially simple .abc files, one static and one animating, that are loaded in Blender as meshes by the
AbcPointsReader
class. The animation is currently possible because the points are represented as meshes that can be loaded by the mesh cache modifier.If you were to load USD points as meshes, you would take a similar approach in implementing a USD points reader class. But the reader implementation would be somewhat different if you were to translate the USD points to Blender point clouds, as is being suggested.
@kevindietrich supports the Alembic IO code. He might have better examples of Alembic point clouds and could provide more information about the Alembic workflow in Blender.
Alembic point cloud import support was supposed to be handled by https://archive.blender.org/developer/D11592 which I recently updated, and will be the subject of a pull request in the very short term I guess. USD point cloud import is missing from the patch, as there is currently no support for point clouds in USD.
There is no reason whatsoever (other than purely ideological ones) for the mesh sequence cache modifier to not support point clouds, and therefore point clouds animation. The name "mesh sequence" can change, it is just historical.
@kevindietrich I wonder if
UsdGeomPoints
can be considered analogous to point clouds for USD. But perhaps there's a distinction I'm not seeing.https://openusd.org/release/api/class_usd_geom_points.html
It's been a minute so I'm trying to get a clear picture of what's changed and what's the same in the USD point cloud import via USDGeomPoint implementation since about 2 months ago. If anything in my attempt to summarize is off, please help correct me since all of this is rather new to me.
The initial direction and goals were
I feel like the additions to this thread lately have been specifically about the support for point clouds that change over time, and I'm unsure whether I understand @kevindietrich when you say
Does this means that we can make this work without the geometry node workflow that @HooglyBoogly is talking about here #106398 (comment).
It seems we also have the question of whether the USDGeomPoints is the right direction to be going in for the
point clouds for USD
as @makowalski describes in the entry above. Perhaps the question here is whether we should be wrapping an existing alembic point cloud importer inside a USD file (perhaps I'm way off).I'm still excited to partake in a contribution, but you may understand my hesitation to start working on anything until it seems clear how we would answer some of these questions.
I just published a branch which updates the patch that I mentioned in my previous post which allows the modifier to output Point Clouds (both for Alembic and USD).
The branch ("abc_usd_geometry_sets") is here https://projects.blender.org/kevindietrich/blender/src/branch/abc_usd_geometry_sets
You can use this branch as a starter. All you have to do is add support for reading a usdGeomPoint as a Blender PointCloud in the USD importer (+ attributes). Animation support is already there, you can simply look up data based on the current time.
usdGeomPoints should be the right object type; my previous comment was a bit cryptic, I meant that our USD implementation does not support point clouds, not USD per se.
Yes. The only thing preventing a modifier to be added to a point cloud object other than a geometry nodes modifier is this condition : https://projects.blender.org/blender/blender/src/branch/main/source/blender/blenkernel/intern/object.cc#L1456
(which is modified in my branch).
Fantastic, thank you so much @kevindietrich !
I feel invigorated to begin with this work. I think I'll start looking through this branch and make sure I come back with questions.
@kevindietrich hi again. I went ahead and tried to get my hands on your branch at https://projects.blender.org/kevindietrich/blender/src/branch/abc_usd_geometry_sets, but my build seems to be failing and I"m not entirely sure if I'm missing a dependency or something.
I'm attaching the build log I created from my screen session here. Unfortunately it missed some of the output from
git pull kev abc_usd_geometry_sets
but you can see the steps I took after that.https://gist.github.com/DevinBayly/73c0f64de0e9bd17ecda8db6a1daa95f
It seems like the above build error stems from adding one too many arguments to the function
BKE_pointcloud_nomain_to_pointcloud
in the filesource/blender/io/alembic/intern/abc_reader_points.cc
. I'm going to remove thetrue
fromI'll just update here that @HooglyBoogly helped me understand in blender chat that the third argument is not necessary.
Onward with the process!
I have been so infrequent in attending to this issue, apologies! I'm interested in picking things back up at this point, has any of the USD Import landscape changed significantly from back a couple months ago? Do I need to refactor my plans at all? Thanks for the info y'all
@devin-bayly There were some minor conflicting changes recently with the branch I shared with you for the starting code but nothing drastic. I updated the branch with the latest changes from main just in case you might need it as a reference again.
@kevindietrich Thanks so much! I recently figured out how to walk the USD stage with a geompoint type in vanilla C++ USD so I feel like the time is ripe to see what happens when I try to incorporate some of that code in Blender following the mechanism laid out in your Alembic importer.
@devin-bayly You shouldn't need to do any of your own stage traversal, as far as I know. That's all taken care of in the framework that's already in place for instantiating USD "readers" in the USD import code. I can help answer any questions you might have about implementing a new USD reader class and could even help prototype the basics of a reader class that would be a starting point for you to add your code, to save you some time. Please let me know if you'd like me to do that.
@makowalski
oh, right! I think we had discussed that before. I think your previous suggestion #106398 (comment) is what I'll do first "checking into some of the other readers". Because then I'll have a better sense of what gets included in reader classes, and how they make use of the code in place for things like traversal.
Definitely will take you up on both of those things probably in about a week, but I want to make sure I have done some self-guided reading so I can use your time more efficiently first.
In the spirit of providing a record of useful information in here I wanted to reflect some information in this issue from @makowalski . I was trying to wrap my head around where the reader classes get instantiated, and the information was provided.
So if I'm following some of the code correctly it looks like perhaps we will be adding a section also to the
create_reader_if_allowed
for the USDGeomPoint type, and then this will call the constructor of the reader class?Yes, that's exactly right! (I can provide a shell implementation of this so you can fill out the rest of the code, as we discussed.)
Great, thanks for the feedback! ok, just slowly unpacking more of the code on my own time. No rush!
Hi @devin-bayly. Starting from Kevin's
abc_usd_geometry_sets
branch, I added a placeholder implementation of aUSDPointsReader
class here:https://projects.blender.org/makowalski/blender/src/branch/abc_usd_geometry_sets
Please see the description for this commit:
84c72a8383
The reader only allocates the point cloud without setting any attributes, but hopefully this will be a reasonable
starting point for your development. You'll mostly need to fill out functionality In
USDPointsReader::read_geometry()
.Please see Kevin's implementation of
AbcPointsReader
for an example of creating the point cloud.I'm also attaching two USDs containing point clouds, one static and one animated, derived from Kevin's ABC point
cloud example.
@kevindietrich for visibility.
Incredible! @makowalski , this is going to launch me forward on this task. Thanks so much for the support.
You're very welcome. And thank you for the initiative you've taken to contribute this new feature. Please let me know if you have questions.
Wow you're outline was like 90% of the code needed. I just got the static usda file to load!
I'm going to try another example from a lidar scan I took. Then I'll try to get the timesampling going which no doubt will have some subtle problems.
Then I'll figure out how to share the code (super small actual contribution at this point haha).
Incredible! Zero other changes and the point cloud updates correctly to whatever time in the animation I'm trying to view!
I guess the next step would be trying to bring in custom attributes like color and such now right?
I should note that the clunkiness of the performance in that video rests squarely with the remote desktop connection I have, and also that I'm using 16 cores on a pretty old HPC node where I've been developing stuff.
@devin-bayly Awesome progress! Nice work! You should share these results on the USD Blender chat channel.
As far as reviewing, I would create a pull request for this, when you feel the time is right. Here are a couple of pages with instructions:
https://wiki.blender.org/wiki/Process/Contributing_Code
https://wiki.blender.org/wiki/Tools/Pull_Requests
Yes, the next step would be supporting attributes. For that, you could take a look at what the USD mesh reader code does. It might make sense to move some of the attribute conversion code out of the mesh reader implementation into shared functions that can be used by the points reader as well, but I haven't thought about this design yet. Initially, as you're experimenting and learning the attributes API, I think it would be fine for you to duplicate the code for the points reader as needed and we can think about consolidating later.
Haha thanks for the praise, it's really all you! I'll definitely follow up on that suggestion to read what the mesh reader is doing for the attributes. I'll also wait on the PR until I've don't a similar thing that you did where you share a link for me to look at your fork of Kevin's work. That way I can get some feedback from you about what I've got written before using anyone's time to review a PR.
I'm going to paste some of the conversations I've been having in the blender chat here
This section can be summarized as my attempts to understand how to access custom primitive information in a usdgeom file.
I wrote the following code to experiment with loading up the custom primitives that I added to a file, but in the text that is printed to the screen for debugging I don't see the custom variables that I've added by hand.
here's the output from running import usd
with this usd file below that I added a double customvar line to. Here's my contribution fork, and the most recent commits are the modifications that show my attempts https://projects.blender.org/devin-bayly/blender/commits/branch/abc_usd_geometry_sets
Is there a different way to iterate over the custom attributes that are present in a USDGeomType ?
I wanted to chime back in to say that
GetProperties
is doing what I was hoping, but I'm now struggling how to get a value out of a property when I've already checked that it is custom. It seems like the USDGetAttribute
thenGet
for a value works but they expect me to declare a time value. https://openusd.org/release/api/class_usd_attribute.html#a9d41bc223be86408ba7d7f74df7c35a9This I think causes a segfault when I attempt to
.Get(0)
on a custom property that doesn't have more than one value.I'm running this code with the following test usda file
@devin-bayly Thanks for the update. I'll have some feedback, suggestions and example code for you tomorrow.
Sounds good, thanks so much!
@devin-bayly As an initial comment, please keep in mind that you will typically be converting USD
primvars
to Blender attributes. I think you added a custom property to the USD, which is different from aprimvar
. Here are a couple of useful links to explain the concepts:https://openusd.org/release/glossary.html#usdglossary-primvar
https://openusd.org/dev/api/class_usd_geom_primvar.html#details
Primvars are array values, where the number of entries depends on the interpolation type (as described in the docs I pointed you to). For example, for
vertex
interpolation, there would be one entry per point. Forconstant
interpolation, the array would have a single entry for the entire point cloud, indicating that every point has the same value. Other interpolation types are possible, depending on the type of geometry.I'm attaching a point cloud dumped from Houdini that has two primvars:
foo
(float) anddisplayColor
(color3f).Here is an example of iterating over the primvars:
I'll try to provide an example of converting primvars in a separate comment, but, as mentioned earlier,
usd_reader_mesh.cc
has many examples of such conversions.Ah yes, this is totally me not understanding the distinction between those things. I also appreciate the quickness you've helped me course correct after I started to look at properties instead.
Thanks for those test files also, I'll add that to my blender development environment and try importing. I think this will also get me over the hump of creating blender attributes from the primvars, and then I can start sharing some more of my code perhaps in the form of a PR (or whatever is appropriate for the first draft of this feature).
Here is the same loop expanded to read color primvars. This is just example code, isn't full tested, and doesn't handle all data and interpolation types, but is hopefully enough to allow you to explore further.
Yes! This was going to be my next question, how to pick the right templated writer provided the types that I get back from the primvar.
This is definitely room to explore, and will help me better understand what I'm reading on the
usd_read_mesh.cc
. Thanks @makowalskiYou're very welcome! As for a PR, we need to keep in mind that this implementation requires changes in Kevin's branch
https://projects.blender.org/kevindietrich/blender/src/branch/abc_usd_geometry_sets
So this PR would need to wait until Kevin has committed these changes to main or we can perhaps create a PR into Kevin's branch instead. We should discuss with @kevindietrich when the time comes, but I don't think we're ready for a PR yet.
Sounds good, as soon as I wrote that I was thinking "I bet that was a hasty suggestion," and will be happy to proceed whatever way makes the most sense!
Stay tuned
Ok, so integrating that code from above I have the color being brought in. I'm curious what the correct name would be to have blender automatically pick up the colors without having to use a custom shading material? Here you can see that I'm using the attribute name
![image](/blender/blender/attachments/82ef4018-a753-4334-952c-7901d82fc4fc)
primvar:displayColor
to make the point colors get selected correctly.Also I'm curious, what other types of interpolation would I be working with besides vertex since with a point cloud we don't have any edges or faces right?
Hi @devin-bayly. Nice progress!
To be honest, I haven't worked with Blender point clouds very much, so I don't know how to specify which color attribute to use for viewport display. Perhaps @HooglyBoogly would know. Otherwise, maybe you can ask on the blender-coders channel.
By the way, you probably want to strip the
primvars:
namespace prefix from the attribute name. I believeUsdGeomPrimvar::StripPrimvarsName()
will do that, if I'm not mistaken:https://openusd.org/release/api/class_usd_geom_primvar.html#a657439a0bca230d608522b786da8bcf3
In addition to
vertex
interpolation, you will encounterconstant
interpolation for point clouds, where a single value is assigned to every point.Point clouds are fairly new in Blender, so they don't have the idea of an "active color attribute" (in Blender lingo) yet. So far they've mostly been created with geometry nodes, where the viewer node is the most convenient answer for previewing a certain attribute anyway.
I wouldn't worry about that for now though, it's just a feature Blender will need to support sometime soon. At that time it can be added to USD I/O as well.
To add to that, Blender currently doesn't have a "constant" storage option, so just fill the entire attribute array with the single value from USD.
It was pretty painless to switch to the color I wanted so I figure I can keep doing it that way for now.
TY for the notes about the interpolations, I'll just behave as if the constants are really redundant values applied to each vertex.
The biggest delay for me right now is actually on the project that motivated this feature. I've been at the step where I can to create a usd file that has a set of custom prim vars for a few attributes on the point cloud provided by the lidar scanner
I'm a bit confused at this point with the python usd library. On this page https://openusd.org/dev/api/class_usd_geom_primvar.html#details it details creating and accessing primvars, but bases it on an existing attribute. I'm stuck at the moment on building my custom primvars for each of the non x,y,z attributes above. Once I have that worked out I'll get back to fitting more pieces into the importer.
OK, that took far too long to work out. I now have the piece in place that will convert my lidar timesteps into usdc. Here's a small snippet of the additional primvars that I'm outputting. I'm seeing something funky when I bring them into blender, even though it appears to read the additional primvars right.
I think perhaps this doesn't relate to what code I'm putting into blender so more work in the preprocessing is required for me still.
I can confirm the usdviewer also has this prob, so I think I can confirm I did something wrong ahead of the blender step. on the right is cloud compare viewing one timestep of this lidar scan.
Ok figured out I left off a transpose somewhere when I was saving out the point arrays.
seems good with color ramps too!
![image](/blender/blender/attachments/38e9c52e-51b9-46b7-8936-c07daa6858e7)
@devin-bayly I just wanted to touch base on how things are going with this task. Looks live you had made some really nice progress! Is there any issue that you need help with?
Hi there!! Yea I'd love to reconnect in a day or so to get a sense of what the best next steps are. Thanks for checking on me!
Shoot how has it been 10 months already
Ok, I'm officially back at work after much vacation. Catching up here I see I should try to work in
primvars:
strippingAfter that I'd be up for a quick go over for my code to hear what I can be doing that improves the quality, or any other ideas about next steps. Let me know what you think @makowalski
That sounds good! I'll be happy to review your code, whenever you're ready. Thanks for the update!
Hi @devin-bayly. Now that the Alembic/USD geometry sets PR has landed, I think the coast is clear to create a PR for the
UsdGeomPoints
import work you've done so far, even if not every feature is complete. You can always continue improving point cloud import after it has landed, if you wish, but I think what you have so far would be quite useful. I'd be happy to help create this initial pull request, if you'd like.@makowalski , I keep starting a reply and then forgetting to send. Sorry! Yes I'm up for what you recommend. I'll just remove the primvars annoyance in the attributes read and then I'll send along to you.
I think I might have botched up a little of the git history so I'll probably need to work on that a little, but here's the changes that I've worked on most recently.
https://projects.blender.org/devin-bayly/blender/src/branch/devin-bayly-patch-3
I think if I go back to my patch-2 branch and then cherry pick just the changes that I made to the points.cc file then I think it'll be a little cleaner what I commit. If you have other suggestions then let me know!
I'm truly in awe now of people who work on the large repos. Even the simple things seem to trip me up. Here's a branch that I think keeps the changes local to the two files (usd_reader_points.cc,.h) that actually got edited. Sorry for all the delays, and thanks for the tremendous amount of support @makowalski '
https://projects.blender.org/devin-bayly/blender/src/branch/devin-bayly-patch-2-clean
what's the next step we should take?
Hi @devin-bayly. I'll take a look at this in the next day or so. I'm sorry for the delay on my part, and thanks for all the hard work!
Hi @devin-bayly. The branch you're working on is very much out of date with main. As a first step, I'd like to apply the changes you have cleanly to top of main so we have a branch we can use to create the pull request. I'm working on this right now and will let you know when the new branch is ready. We can then continue the review from there.
Hi @devin-bayly . I made this branch that has your changes from
devin-bayly-patch-2-clean
applied to the latestmain
:https://projects.blender.org/makowalski/blender/src/branch/usd-points-import
I made some updates to get it to compile with
main
. I also renamedusd_reader_points.h
tousd_reader_points.hh
, since the .hh extension is now standard in the USD module. In addition, I did some minor cleanup:Would you mind creating a pull request from this branch? Basically, if you clone my
usd-points-import
branch and push it to your own remote, you'll then have an option to create a pull request from it. This will allow us to continue with the review process and eventually merge it into main.Here is some info on creating pull requests: https://developer.blender.org/docs/handbook/contributing/pull_requests/
Please let me know if you have questions.
Oh i bet, thanks for helping out with making the code apply to the latest version of the source.
Happy to make a PR from that branch and I'll quickly read over that link below.
@makowalski Just checking, did I do what you were hoping here? https://projects.blender.org/devin-bayly/blender/src/branch/usd-points-import
Hi @devin-bayly. Thanks for following up! I see your copy of the branch, but did you create the pull request?
https://developer.blender.org/docs/handbook/contributing/pull_requests/#create-a-pull-request
Haha yea I only did the copying step, but I'll submit the pr now. It's going to
blender:main
hope that's rightOh, it looks like
makowalski:main
is the only option I have in the drop downYes, it should go to blender:main. I'm not sure why you're not seeing it. Maybe something went wrong when you copied my branch.
How did you copy my branch?
In any case, maybe try copying the usd-import-points branch to another name locally, push it to your remote, and see if the pull works in that case?
haha I knew I messed something up a bit. So my branch cloning worked something like,
Does this go further back to the point where I first cloned the repo using your page instead of the original blender source repo?
Yes, it could be that you did not fork the original blender source repo.
Did you originally follow this setup: https://developer.blender.org/docs/handbook/contributing/pull_requests/#one-time-setup
@devin-bayly Per our direct discussion yesterday, I opened pull request #120060 based on your branch.
Support is now available in
main
with commite4ef0f6ff4