T99807: Add support for exporting to USDZ #104556
No reviewers
Michael Kowalski
Sybren A. Stüvel
Bastien Montagne
Labels
No Label
Interest
Alembic
Interest
Animation & Rigging
Interest
Asset Browser
Interest
Asset Browser Project Overview
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
EEVEE & Viewport
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
Virtual Reality
Interest
Vulkan
Interest
Wayland
Interest
Workbench
Interest: X11
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
EEVEE & Viewport
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
Platform
FreeBSD
Platform
Linux
Platform
macOS
Platform
Windows
Priority
High
Priority
Low
Priority
Normal
Priority
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
3 Participants
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: blender/blender#104556
Loading…
Reference in New Issue
No description provided.
Delete Branch "SonnyCampbell_Unity/blender:unity/T99807-usdz-refactor"
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?
Historical patch review here: https://archive.blender.org/developer/D15623
This change will add the default .usdz export capability. There is a separate UsdUtilsCreateNewARKitUsdzPackage capability for exporting usdz for iOS devices that will be implemented in a follow up patch as it will require some more small UI changes.
The importer already supports usdz so no change is required other than updating the text in menu to match the updated exporter text.
On export, we must first create a .usd/a/c file and then covert it to .usdz. We are required to do a weird change of directory because of a quirk with the USD conversion to usdz. If an absolute filepath is passed into the UsdUtilsCreateNewUsdzPackage function, the usd files inside the usdz archive will have the same directory structure, i.e. if I try to create a file at C:\code\BlenderProjects\file.usdz, when I create the usdz file inside it will have the structure \code\BlenderProjects\file.usdc.
This is counteracted by setting the current working directory to the temporary session directory where I create both the usdc and usdz files, and just passing the file name to UsdUtilsCreateNewUsdzPackage without any filepath. Once the usdz file is created I copy it out to the intended directory.
Multiframe copy paste operations were prohibited by a block of code. While this is understandable as it prevents confusing actions, there is a good use case to allow multiframe copy pasting. This patch does: - remove restricting block of code - adds outer loops for frames both in copy and paste actions. In copy action it will create an array of selected strokes from all selected frames In paste action it will create new from previous copy action merged to a single frame. It will paste to all selected frames and create new frame if autokey is on and time cursor is not on any of the keyframes. The main usecase is for creating motion smears as in examples attached. The user still need to do some actions in order to achieve desired look, such as stroke ordering and deleting excessive strokes. {F14069747} {F14069743} Reviewed By: antoniov, mendio Differential Revision: https://developer.blender.org/D16805This was the case when an Annotation is also present as in the report. Since {rB92d7f9ac56e0}, Dopesheet is aware of greaspencil/annotations (displays its channels/keyframes, can rename their data, layers, fcurve groupings etc.). However, the Graph Editor does not display these / should not be aware of them. Above commit already had issues that were addressed in the following commits (mostly adding the new `ANIMFILTER_FCURVESONLY` to places that dont handle greasepencil/annotations) - {rBfdf34666f00f} - {rB45f483681fef} - {rBa26038ff3851} Now in T103303 it was reported that doublicking a channel would not select all fcurve`s keyframes anymore and this wasnt actually an issue with that particular operator, but instead with another operator that is also mapped to doubleclicking: the channel renaming (I assume the keyconflict here is actually wanted behavior). Channel renaming would not return `OPERATOR_PASS_THROUGH` here anymore in the case nothing cannot be renamed, so we would not actually reach the 2nd operator at all (the one selecting all keyframes). Deeper reason for this is that the renaming operator would actually "see" a channel that could be renamed (in the case of the report: an annotation layer), but then cannot proceed, because the "real" underlying data where the doubleclick happens is an fcurve... So now exclude non-greasepencil channels in filtering as well where appropriate by also adding `ANIMFILTER_FCURVESONLY` to the filter there. Fixes T103303. Maniphest Tasks: T103303 Differential Revision: https://developer.blender.org/D16871This patch adds two new panels in which users can control offset (position, rotation, scale) by layer and by stroke number. Use cases: - if you want to create array of Suzannes and place them one behind another, in 2d mode, Z depth will be wrong. In 3d mode there will be Z fighting. With combination of stroke and layer offset we can fight this issue without touching original drawing. - even easier way to create 2.5d environments.. simply draw on layers and then displace them in one panel instead fiddling with every layer individually Possible improvements: - add offset by material slot - add step parameter, to allow displacing groups of N strokes. {F13129598} Reviewed By: mendio, antoniov Differential Revision: https://developer.blender.org/D1510663b9a57f8b**Changes** As described in T93602, this patch removes all use of the `MVert` struct, replacing it with a generic named attribute with the name `"position"`, consistent with other geometry types. Variable names have been changed from `verts` to `positions`, to align with the attribute name and the more generic design (positions are not vertices, they are just an attribute stored on the point domain). This change is made possible by previous commits that moved all other data out of `MVert` to runtime data or other generic attributes. What remains is mostly a simple type change. Though, the type still shows up 859 times, so the patch is quite large. One compromise is that now `CD_MASK_BAREMESH` now contains `CD_PROP_FLOAT3`. With the general move towards generic attributes over custom data types, we are removing use of these type masks anyway. **Benefits** The most obvious benefit is reduced memory usage and the benefits that brings in memory-bound situations. `float3` is only 3 bytes, in comparison to `MVert` which was 4. When there are millions of vertices this starts to matter more. The other benefits come from using a more generic type. Instead of writing algorithms specifically for `MVert`, code can just use arrays of vectors. This will allow eliminating many temporary arrays or wrappers used to extract positions. Many possible improvements aren't implemented in this patch, though I did switch simplify or remove the process of creating temporary position arrays in a few places. The design clarity that "positions are just another attribute" brings allows removing explicit copying of vertices in some procedural operations-- they are just processed like most other attributes. **Performance** This touches so many areas that it's hard to benchmark exhaustively, but I observed some areas as examples. * The mesh line node with 4 million count was 1.5x (8ms to 12ms) faster. * The Spring splash screen went from ~4.3 to ~4.5 fps. * The subdivision surface modifier/node was slightly faster RNA access through Python may be slightly slower, since now we need a name lookup instead of just a custom data type lookup for each index. **Future Improvements** * Remove uses of "vert_coords" functions: * `BKE_mesh_vert_coords_alloc` * `BKE_mesh_vert_coords_get` * `BKE_mesh_vert_coords_apply{_with_mat4}` * Remove more hidden copying of positions * General simplification now possible in many areas * Convert more code to C++ to use `float3` instead of `float[3]` * Currently `reinterpret_cast` is used for those C-API functions Differential Revision: https://developer.blender.org/D15982Related to {T103758}. - NSStringPboardType is replaced by NSPasteboardTypeString - NSTIFFPboardType is replaced by NSPasteboardTypeTIFFCaused by {rBd397ecae325}. Above commit added a new socket, so `version_geometry_nodes_primitive_uv_maps` was getting the wrong sockect with `->next`. Now get the right one with yet another `->next` (might not be ideal, but searching the right socket with other methods might be overhead?) Maniphest Tasks: T103837 Differential Revision: https://developer.blender.org/D16994`std::get` could not be used due to restrictions on macos. However, the minimum requirement has been lifted in {rB597aecc01644f0063fa4545dabadc5f73387e3d3}.This wasn't really a problem since these are set on first bind or creation. The test `if (enabled_srgb && srgb_) {` was depending on that variable that in certain case, might not have been initialized (because of lazy init).This fixes the UI alignment issues that were introduced by {D12815} with the addition of the boolean custom properties. Reviewed By: HooglyBoogly Differential Revision: https://developer.blender.org/D17012Goal of this patch is to stop the invocation of OpenGL calls via the bgl module on a none OpenGL GPU backend, report this as a python deprecation warning and report this to the user. ## Deprecation warning to developers ``` >>> import bgl >>> bgl.glUseProgram(0) <blender_console>:1: DeprecationWarning: 'bgl.glUseProgram' is deprecated and will be removed in Blender 3.7. Report or update your script to use 'gpu' module. ``` ## Deprecation message to users The message to the user is shown as part of the Info Space and as a message box. {F14159203 width=100%} {F14158674 width=100%} During implementation we tried several ideas: # Use python warning as errors: This isn't fine grained enough and can show incorrect information to the user. # Throw deprecation as error and use sys.excepthook to report the user message. This required a custom exception class to identify the bgl deprecation and a CPython handler function to be set during python initialization. Although this is the most flexible there was a disconnect between the exception class, exception function and the excepthook registration. # A variant how we handle autoexec failures. A flag is stored in Global and when set the user message is reported. Not that flexible, but code is more connected to the boolean stored in the Global struct. Although using Global struct isn't nice I chose this solution due to its traceability. It is clear to developers reading the code how the mechanism works by using search all functionality of your IDE. Reviewed By: MichaelPW, campbellbarton Maniphest Tasks: T103863 Differential Revision: https://developer.blender.org/D16996This was the case when using the operator outside of the modifiers panel. Caused by {rBeae36be372a6}. In above commit, `DT_layer_items` shared both `DT_TYPE_MPROPCOL_LOOP` | `DT_TYPE_MLOOPCOL_LOOP` in a single EnumPropertyItem value "Colors". This is a bit unusual, but probably allowed. As a consequence, checks for specific datatypes would fail when selecting such EnumPropertyItem: - `DT_DATATYPE_IS_MULTILAYERS` (uses `ELEM` to check distinct entries -- would return false) - `BKE_object_data_transfer_dttype_to_srcdst_index` (would return `DT_MULTILAYER_INDEX_INVALID`) These places have now been corrected to take these "special" values into account. Another issue was that multiple EnumPropertyItems with the same value could be created in dt_add_vcol_layers() if attributes of the same domain, but different color types are in play (could lead to crashes) and that has also been corrected. Also: above commit did not give the choice of transfering color attributes from the vertex domain (only face corner attributes could be chosen), this has now been added. DT_layer_vert_items (used from the modifier) already had this included so this was only an issue when using the operator outside of the modifiers panel. Since we now feature two domains, the single "VCOL" in the enum has been split into "COLOR_VERTEX" and "COLOR_CORNER". This will break existing scripts calling bpy.ops.object.datalayout_transfer and will be marked as a breaking change in the release notes. NOTE: there is another bug here when attributes of the same domain, but different color types are in play and you want to transfer just a single specific layer (but that is for a separate commit) Maniphest Tasks: T103400 Differential Revision: https://developer.blender.org/D16935Previously, transforming a clip (scaling, repeat, etc) wouldn't re-calculate the blend-in and blend-out values, leading to over / undershoot, and a visual clip artifact Old: {F14045003} This patch adds re-calculation logic (new `BKE_nlastrip_recalculate_blend()` method) to the blend-in/out on transformations to clamp values, and avoid over/under shoot. The `BKE_nlastrip_recalculate_blend()` encapsulates the existing logic for both the `rna_NlaStrip_blend_in_set()` and `rna_NlaStrip_blend_out_set()` methods into a single BKE method that we an execute as needed. The fact that blend-in is first decreased, is strictly on the order of calculation. My suspicion is that //if// the blend-in / blend-our values were working as intended, the RNA set methods would update in order, and we'd experience the same thing. In short, the choice here was to linearly combine the logic, without making any assumptions of my own. while talking things over with @sybren and @RiggingDojo, they are fine with how this currently works, but there a desire to update how the two values interact with each (ratio scale, etc) in the future. New: {F14045024} {F14045025} Reviewed By: sybren Maniphest Tasks: T101369 Differential Revision: https://developer.blender.org/D16720Caused by {rB841df831e89d} and {rB3558bb8eae75}. These commits moved flags from `eLineArtGPencilModifierFlags` to `eLineartMainFlags`, but later on in code, these were still evaluated from the modifiers `flags` (instead of `calculation_flags`). This resulted in a false condition (`match_output` was assumed true but it wasnt), leading to a wrong codepath taken. This is now corrected (`calculation_flags` need to be passed around for this as well). Maniphest Tasks: T103887 Differential Revision: https://developer.blender.org/D17062This patch fixes an issue where Blender 3.5 alpha with the Metal GPU backend enabled on Japanese macOS fails to compile shaders and crashes on startup. In a Japanese environment, `defaultCStringEncoding` is the legacy MacJapanese encoding, and it erroneously converts backslashes (0x5c) to Yen symbols (¥). Therefore, Metal shader compile fails with the following log and Blender crashes. ``` 2022-12-29 13:50:10.200 Blender[13404:246707] Compile Error - Metal Shader Library (Stage: 0), error Error Domain=MTLLibraryErrorDomain Code=3 "program_source:225:74: error: non-ASCII characters are not allowed outside of literals and identifiers template<typename T, access A = access::sample> struct STRUCT_NAME { ¥ ^ program_source:226:14: error: no template named 'TEX_TYPE' thread TEX_TYPE<T, A> *texture; ¥ ^ program_source:226:39: error: non-ASCII characters are not allowed outside of literals and identifiers thread TEX_TYPE<T, A> *texture; ¥ ^ program_source:227:29: error: non-ASCII characters are not allowed outside of literals and identifiers thread sampler *samp; ¥ ^ ... ``` We can use `stringWithUTF8String` instead. Reviewed By: fclem, MichaelPW Differential Revision: https://developer.blender.org/D16881This adds a new operator: bpy.ops.mesh.flip_quad_tessellation() This operator rotates the internal loops of the selected quads, allowing the user to control tessellation without destructively altering the mesh. {F14201995} This operator can be found in the "Face" menu (Ctrl+F) under "Face Data". {F14201997} Reviewed By: campbellbarton, dbystedt Differential Revision: https://developer.blender.org/D17056Extract: - EEVEE: Compiling Shaders (the same message exists in EEVEE Next, but it uses string concatenation and I don't know yet how to deal with those--see T92758) Disambiguate: - Pan (audio, camera) - Box (TextSequence) - Mix (noun in constraints, GP materials) - Volume (object type, file system) - Floor (math integer part, 3D viewport horizontal plane) - Impossible to disambiguate the constraint name because bConstraintTypeInfo doesn't have a context field. - Show Overlay (in the sequence editor, use the same message as other editors to avoid a confusion with the Frame Overlay feature, also called "Show Overlay") Additionally, fix a few issues reported by Joan Pujolar (@jpujolar) in T101830. Reviewed By: mont29 Differential Revision: https://developer.blender.org/D17114Regression in {d0f55aa671f7}Was using squared cosine instead of cosine. The problem is obvious for large spread angles. |before|after |{F14220946}|{F14220948} The images are generating by returning `floor(spotmask)` in function `spot_attenuation()` in `lights_lib.glsl`.Even though the dope sheet respects the pinned channels, it did not show the icon to interact with the pinned state. This adds the icon to the dope sheet as well. {F14178715} Reviewed by: Sybren A. Stüvel Differential Revision: https://developer.blender.org/D17061 Ref: D17061Cycles ignores the size of spot lights, therefore the illuminated area doesn't match the gizmo. This patch resolves this discrepancy. | Before (Cycles) | After (Cycles) | Eevee |{F14200605}|{F14200595}|{F14200600}| This is done by scaling the ray direction by the size of the cone. The implementation of `spot_light_attenuation()` in `spot.h` matches `spot_attenuation()` in `lights_lib.glsl`. **Test file**: {F14200728} Differential Revision: https://developer.blender.org/D17129This patch also contains the changes from #104525 because it is dependent on the logic to change working directory.
I spoke to Dhruv about the last comment in https://archive.blender.org/developer/D15623 as I was unable to replicate the behaviour he was getting, so it is something we will investigate going forward.
Nice work, just some minor nags.
@ -49,0 +51,4 @@const char *export_filepath() const{if (strlen(usdz_filepath) > 0) {No need to count all the characters when all you want to know is "empty or not". Use either
usdz_filepath[0]orBLI_strnlen(usdz_filepath, 1) > 0.This is utterly incorrect,
usdz_filepathis alwaystrueby definition... you want to checkusdz_filepath[0] != '\0'here...But that whole function can simply be
return usdz_filepath[0] != '\0';I think the null terminator always evaluates to false.
So if you have an empty string with
usdz_filepath[0] = '\0'thenif(usdz_filepath[0])will evaluate to false.The check for
if(usdz_filepath)is to make sure that theusdz_filepathhas been initialised.Though maybe your way is more explicit and there's less possibility for confusion?
Though yeah I just did a check and it looks like the
if(usdz_filepath)is unnecessary so I'll remove it.@ -59,0 +90,4 @@pxr::UsdUtilsCreateNewUsdzPackage(pxr::SdfAssetPath(usdc_file), usdz_file);BLI_change_working_dir(original_working_dir);char usdz_temp_dirfile[FILE_MAX];What is a dirfile? I think a term like "full path" would be better than "dirfile".
@ -59,0 +102,4 @@return false;}}result = BLI_rename(usdz_temp_dirfile, data->usdz_filepath);Is it guaranteed that the temp "dirfile" is on the same filesystem as
usdz_filepath? Have you tested with them being on different filesystems?@ -159,19 +218,50 @@ static void export_startjob(void *customdata,BKE_scene_graph_update_for_newframe(data->depsgraph);}if (strlen(data->usdz_filepath) > 0) {Same as above about counting all the characters.
Now that this logic is used here as well, I actually think that this shows this code is sitting at the wrong abstraction level. What is being queried here is "should this output USD or USDZ?", and the fact that that distinction is made by whether
usdz_filepathis empty or not is an implementation detail.Something like
bool ExportJobData::targets_usdz() { return usdz_filepath[0]; }would solve both problems.@ -160,2 +219,4 @@}if (strlen(data->usdz_filepath) > 0) {if (!perform_usdz_conversion(data)) {This could just be an
&&in the sameif.As a personal preference I prefer to have functions that actually do logic outside of
if()conditions as I find it can be quite easy to miss, so I've moved it to the line above and stored the result in a variable that gets checked.I can do it like you suggest if you wish, but I find it much more readable this way.
@ -166,1 +234,4 @@static void export_endjob_usdz_cleanup(const ExportJobData *data){if (!BLI_exists(data->unarchived_filepath)) {Why should this not clean up anything if
unarchived_filepathis missing? Wouldn't it be prudent to always eraseusdc_temp_dir?usdc_temp_diris the directory that containsdata->unarchived_filepath, so if there is nodata->unarchived_filepaththere shouldn't be anything to delete.This is basically so that we don't try to delete anything when there shouldn't be anything there to delete in the first place.
@ -185,1 +275,4 @@/* To create a usdz file, we must first create a .usd/a/c file and then covert it to .usdz. The* temporary files will be created in Blender's temporary session storage. The .usdz file will then* copied to job->usdz_filepath. */This is not what the code above is doing -- it's moving, not copying.
@ -186,0 +292,4 @@MEM_freeN(usdc_file);}static void set_job_filepath(const char *filepath, blender::io::usd::ExportJobData *job)Most setters have the parameter order to mimick
thing = value, so are in the formsetter(thing, value). Swap the two parameters here.@ -0,0 +62,4 @@BKE_tempdir_init(temp_dir);const std::string &test_assets_dir = blender::tests::flags_test_asset_dir();BLI_path_join(output_filename, FILE_MAX, test_assets_dir.c_str(), "usd/output_новый.usdz");👍 for testing with unicode characters.
Using the assets dir is not that great, though -- same as in #104525. Since this code already uses
BKE_tempdir_init(), you can useBKE_tempdir_base();andstd::tmpnam()to generate a name that's guaranteed to be unique.@ -0,0 +73,4 @@if (BLI_exists(output_filename)) {BLI_delete(output_filename, false, false);BLI_delete(temp_dir, true, true);Why not always delete
temp_dir, regardless of whetheroutput_filenaemexists or not?@ -0,0 +86,4 @@}/* File sanity check. */EXPECT_EQ(BLI_listbase_count(&bfile->main->objects), 4);Use
ASSERT_EQfor such checks. That will stop the test from continuing when it fails.@ -0,0 +91,4 @@USDExportParams params{};bool result = USD_export(context, output_filename, ¶ms, false);EXPECT_TRUE(result);Add some logging to this and the following EXPECT calls. When the test fails, it should be clear why it failed. You can do something like:
If this particular test fails, the rest of the test is useless (it will all fail), so this one can be replaced with
ASSERT_TRUE.Nice work, just some minor nags.
@blender-bot build
@blender-bot build
Issues noted below are fairly bad (as in, obvious mistakes and potential source of bugs), would be good to check at compiler warnings when building...
Hopefully these are the only serious ones, will fix them (since it's trivial) as part of the commit.
For the rest tests are passing and feature looks good, so time to finalize this patch.
@ -59,0 +93,4 @@BLI_split_file_part(data->usdz_filepath, usdz_file, FILE_MAX);char original_working_dir[FILE_MAX];BLI_current_working_dir(original_working_dir, FILE_MAX);This line and the one above are not properly using the API, which expect you to use the returned char pointer.
@ -0,0 +56,4 @@}char original_wd[FILE_MAX];BLI_current_working_dir(original_wd, FILE_MAX);Same issue as noted above regarding not using return value from
BLI_current_working_dir().Realised the
usdz_export_test.blendtest blend file hasn't been uploaded yet.Committed as
54f672cbfafrom PR #105185.Pull request closed