Nodes: Merge Musgrave Texture Node Into Noise Texture Node #111187
No reviewers
Omar Emara
Brecht Van Lommel
Jacques Lucke
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
13 Participants
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: blender/blender#111187
Loading…
Reference in New Issue
No description provided.
Delete Branch "Hoshinova/blender:merge-noise-nodes2"
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?
A continuation of #111053
The PR corresponding to https://devtalk.blender.org/t/merging-the-musgrave-texture-and-noise-texture-nodes/30646
This PR merges all functionalities of the Musgrave Texture node into the Noise Texture node.
Apart from removing duplicate functionalities and a few thousand lines of duplicate code, it effectively adds the Distortion input and the Color output to the formerly Musgrave Texture exclusive noise types (Multifractal, Ridged Multifractal, Hybrid Multifractal and Hetero Terrain).
The Noise Texture output corresponds to the Musgrave Texture fBM noise type.
The Dimension input of the Musgrave Texture corresponds to the Roughness input of the Noise Texture and can be converted as follows: $Roughness=Lacunarity^{-Dimension}$
Using Roughness instead of the Dimension input has the added advantage of fixing #112180 for all Musgrave Texture noise types, thus completing #112962
Because
Lacunarity^{-Dimension}can produce values greater than 1.0 the Roughness input of the new Noise Texture will no longer be clamped to [0.0, 1.0] but instead to [0.0, $\infty$).This allows artists to create "rougher" noise than was previously possible.
The versioning implemented in this PR ensures that the Dimension inputs are correctly converted to their respective Roughness values for all Musgrave Texture nodes and all Roughness values are correctly clamped for all Noise Texture nodes.
The Detail inputs of the Musgrave and Noise Textures despite sharing the same name unfortunately do not behave identically.
The Musgrave Texture Detail input computes one octave fewer than the Noise Texture Detail input.
That is $Detail_{Noise}=Detail_{Musgrave} -1.0$
Problems arise when
Detail_{Musgrave}<1.0as there is no correspondingDetail_{Noise}value because the Detail values are always non-negative.The fBM and Hybrid Multifractal noise types are linear as functions of the Detail input when
Detail<1.0meaning it's possible to just scale the new outputs to match the old ones.While not linear the Multifractal noise type is affine linear as a function of the Detail input when
Detail<1.0meaning it's possible to just scale the new outputs then add a constant offset to match the old ones.The Ridged Multifractal and Hetero Terrain noise types aren't even affine linear making it much more difficult to match the new and old outputs. Unless... There is a bug that went undiscovered for years that makes the Musgrave Texture node to skip evaluation of the first octave. That's right, there is a bug that effectively causes the Musgrave Texture node to have the same output when the Detail is set to 0.x and 1.x, meaning that for the case
Detail_{Musgrave}<1.0we can simply substituteDetail_{Noise}=Detail_{Musgrave}avoiding the problem altogether.As for the bug I mentioned this PR fixes it as well by simply using
Detail_{Noise}instead ofDetail_{Musgrave}.The versioning implemented in this PR ensures that the
Detail_{Musgrave}inputs are correctly converted to their respectiveDetail_{Noise}values for Musgrave Texture nodes.Forward compatibility with Blender 4.0 is ensured by #114236.
Forward compatibility with Blender 3.6 LTS is ensured by #115015.
The output of the Noise and Musgrave Texture nodes in Blender 3.6 LTS and 4.0 is usually identical to that in Blender 4.1, but with a few exceptions:
Detail_{Musgrave}=Detail_{Noise} + 1.0however when the Detail input in Blender 4.1 is set to a value>14.0it will be clamped to a maximum of15.0. This happens internally and can't be circumvented using nodes.10^{-5}.1.0.Below is an image of the Noise Texture node. Left is the old version, right is the new version.
@blender-bot package
Package build started. Download here when ready.
@ -397,0 +425,4 @@bNode *subNode = nodeAddStaticNode(nullptr, ntree, SH_NODE_MATH);subNode->custom1 = NODE_MATH_SUBTRACT;subNode->locx = node->locx;Node location is off when node is inside node frame
@blender-bot package
Package build started. Download here when ready.
@blender-bot package
Package build started. Download here when ready.
Merge Musgrave Texture Node Into Noise Texture Nodeto Nodes: Merge Musgrave Texture Node Into Noise Texture Node@blender-bot package
Package build started. Download here when ready.
@blender-bot package
Package build started. Download here when ready.
A .blend file for testing the whether all shader node implementations are equivalent is here:
Noise_Test_File.blend
I still haven't found a good solution for testing Geo nodes, since the Geo nodes viewer only allows viewing one object at a time.
So Jacques you can get a little creative testing the Geo nodes implementation on your side. 😉
I am skeptical on the use of
socket->link->fromnodein the versioning code, as I am not sure when/if they get updated. It seems a loop over links is needed. Maybe @JacquesLucke knows for sure?Main should be merged and the subversion should be updated, other than that, looks good.
@ -408,0 +412,4 @@if (node->type == SH_NODE_TEX_MUSGRAVE_DEPRECATED) {STRNCPY(node->idname, "ShaderNodeTexNoise");node->type = SH_NODE_TEX_NOISE;NodeTexNoise *data = (NodeTexNoise *)MEM_callocN(sizeof(NodeTexNoise), __func__);MEM_cnew<NodeTexNoise>What's the difference?
You should not use this if there is a specific equivalent. Code cleanliness.
So it's the same?
@ -408,0 +413,4 @@STRNCPY(node->idname, "ShaderNodeTexNoise");node->type = SH_NODE_TEX_NOISE;NodeTexNoise *data = (NodeTexNoise *)MEM_callocN(sizeof(NodeTexNoise), __func__);data->base = ((NodeTexMusgrave *)node->storage)->base;Use
static_cast<NodeTexMusgrave *>()for allvoid *->T *conversions@ -408,0 +440,4 @@subNode1->locx = node->locx;subNode1->locy = node->locy - 300.0f;subNode1->flag |= NODE_HIDDEN;bNodeSocket *subSock1A = static_cast<bNodeSocket *>(BLI_findlink(&subNode1->inputs, 0));nodeFindSocket?I asked and apparently it's the same.
Code cleanliness.
I know but using
static_cast<bNodeSocket *>(BLI_findlink())seems to be most common for the math node for some reason. See:0edd60b4c7/source/blender/blenloader/intern/versioning_280.cc (L2136)and
2419acf756/source/blender/blenloader/intern/versioning_280.cc (L1311).Right,
socket->link->fromnodeshouldn't be used, unless it has been set explicitly before. Not sure how often we use that in versioning code currently. If we don't use it much already, better create a new temporaryMapinstead of usingfromnode.The versioning code should be updated to not assume updated links then.
Today marks the historical day were @mod_moder and I agree for once.
MEM_cnewwill be used instead ofMEM_callocN!@OmarEmaraDev @JacquesLucke I've changed the versioning code to not use
socket->link->fromnode.Unrelated to this PR, but the versioning function below mine
4aaae8d0a6/source/blender/blenloader/intern/versioning_400.cc (L638)usessocket->link->fromnodeextensively.Might want to give a note to @LukasStockner that it shouldn't be used.
A .blend file for testing the versioning is here:
[Versioning_Test_File.blend]
A .blend file for testing the whether all node implementations are equivalent is here:
[Noise_Test_File.blend]
Note: Both files are outdated, please use the files provided in #111187 (comment)
@blender-bot package
Package build started. Download here when ready.
@ -583,0 +592,4 @@data->dimensions = (static_cast<NodeTexMusgrave *>(node->storage))->dimensions;data->normalize = false;data->type = (static_cast<NodeTexMusgrave *>(node->storage))->musgrave_type;node->storage = data;Does the old
node->storageneed to be freed?@blender-bot package
Package build started. Download here when ready.
@blender-bot build
@blender-bot package
Package build started. Download here when ready.
@ -588,0 +591,4 @@LISTBASE_FOREACH (bNode *, node, &ntree->nodes) {if (node->type == SH_NODE_TEX_MUSGRAVE_DEPRECATED) {STRNCPY(node->idname, "ShaderNodeTexNoise");node->type = VERSIONING_NODE_1;I don't understand why this is necessary. The loops where you use that value to identify the node can just compare the
nodepointer from the upper loop to thelink->tonodepointer.@ -588,0 +597,4 @@data->dimensions = (static_cast<NodeTexMusgrave *>(node->storage))->dimensions;data->normalize = false;data->type = (static_cast<NodeTexMusgrave *>(node->storage))->musgrave_type;node->storage = data;Yes, do free the old storage first.
@ -588,0 +603,4 @@float *detail = version_cycles_node_socket_float_value(sockDetail);bNodeSocket *sockFac = nodeFindSocket(node, SOCK_OUT, "Fac");if (sockFac->label[0] != '\0') {The if statement is redundant in this case. Add a comment saying why the label is cleared.
@ -588,0 +610,4 @@uint8_t noise_type = (static_cast<NodeTexNoise *>(node->storage))->type;float locyoffset = 0.0f;if (version_node_socket_is_used(sockDetail) && sockDetail->link != nullptr) {You are still using
socket->linkin multiple parts of the code. The link itself is the one that can be non-updated, not itstoandfromnodes members. Sorry for the misunderstanding.Consider the following code:
So you essentially find all sockets at once, then handle them after. You can even do it for all nodes in the same loop with a bit more work.
@blender-bot package
Package build started. Download here when ready.
@ -588,0 +597,4 @@data->dimensions = (static_cast<NodeTexMusgrave *>(node->storage))->dimensions;data->normalize = false;data->type = (static_cast<NodeTexMusgrave *>(node->storage))->musgrave_type;MEM_freeN(node->storage);Just to make sure
MEM_freeN(node->storage);is enough to free the memory right? No need to tell it how much memory to free etc.?Yes, those things are stored internally so you shouldn't worry about them.
@OmarEmaraDev @JacquesLucke @brecht I've updated the test .blend files, please use the new ones.
@blender-bot package
Package build started. Download here when ready.
An updated .blend file for testing the versioning is here:
Versioning_Test_File.blend
It was originally created in Blender 3.4 and contains Musgrave, Noise and Wave Texture Materials.
When loaded into 4.0 the Musgrave Textures should automatically get converted to the corresponding Noise Textures.
The output (rendered image) should be identical in both versions.
A .blend file for testing the whether all node implementations are equivalent is here:
Noise_Test_File.blend
And just for you @JacquesLucke I even looked into how Geometry nodes work 😉 .
On top of the Shader nodes test planes are a layer of test planes passing the Geometry node Noise texture outputs as a color attribute to the Material output. Simply toggle the "Geo Nodes" Collection in the Outliner to switch between the Shader and Geometry nodes test planes.
Apart from the fact that the Geometry nodes version is more blurry because it's only evaluated at mesh vertices the output (rendered image) should be identical for all node implementations.
@ -595,0 +611,4 @@}}bNodeSocket *sockRoughness = nodeFindSocket(node, SOCK_IN, "Roughness");nullptr
@blender-bot package
Package build started. Download here when ready.
@blender-bot package
Package build started. Download here when ready.
@blender-bot package
Only blender organization members with write access can start builds. See documentation for details.
@blender-bot package
Only blender organization members with write access can start builds. See documentation for details.
@blender-bot package
Package build started. Download here when ready.
Please run
make format, there are some formatting issues inversioning_400.cc.Looks like every old
Musgravenode now has at least two additional nodes (MultiplyandPower). This seems unnecessary when we can just change the roughness value inplace.@ -535,3 +535,2 @@template<typename T>float perlin_fractal_template(T position, float octaves, float roughness, float lacunarity, bool normalize)float perlin_fbm(Would be nice to mention what
fbmstands for here in a comment.@ -764,3 +563,1 @@/* -------------------------------------------------------------------- *//** \name Musgrave Noise* \{ *//* Explicilt instantiation for Wave Texture. */typo
@ -596,0 +600,4 @@if (node->type == SH_NODE_TEX_NOISE) {(static_cast<NodeTexNoise *>(node->storage))->type = SHD_NOISE_FBM;bNodeSocket *sockRoughness = nodeFindSocket(node, SOCK_IN, "Roughness");roughness_socketYou mean I should rename
sockRoughnesstoroughness_socket? Why just this specific socket? It makes the naming inconsistent with the rest of the code.https://wiki.blender.org/wiki/Style_Guide/C_Cpp#Naming
Yeah, I know but a lot of other versioning code also uses Camel case. Should I change all variable names to snake case?
No, do this only for new code. All unrelated changes can be sent as separate cleanups.
@JacquesLucke You sure that I should rename all new variables? (It's quite a lot)
Yes, new code should use the snake case naming conventions mentioned in the style guide.
@ -596,0 +612,4 @@bNode *roughnessFromNode = nullptr;bNodeSocket *roughnessFromSock = nullptr;LISTBASE_FOREACH_BACKWARD_MUTABLE (bNodeLink *, link, &ntree->links) {Any reason for using
LISTBASE_FOREACH_BACKWARD_MUTABLEinstead ofLISTBASE_FOREACH.I think @OmarEmaraDev told me to.
That was for the case when were adding and removing links inside the loop, hence the mutable and backward keywords.
@ -596,0 +614,4 @@LISTBASE_FOREACH_BACKWARD_MUTABLE (bNodeLink *, link, &ntree->links) {/* Find links, nodes and sockets. */if ((link->tonode == node) && (STREQ(link->tosock->identifier, "Roughness"))) {Couldn't you compare
link->tosockwithsockRoughness?Thought about that, but kept it like this to make the logic consistent with
versioning_replace_musgrave_texture_node. But will change if you want that.@ -596,0 +655,4 @@{version_node_input_socket_name(ntree, SH_NODE_TEX_MUSGRAVE_DEPRECATED, "Dimension", "Roughness");LISTBASE_FOREACH (bNode *, node, &ntree->nodes) {if (node->type == SH_NODE_TEX_MUSGRAVE_DEPRECATED) {continueearly for the wrong node type, instead of indenting everything below.@ -596,0 +678,4 @@bNode *lacunarityFromNode = nullptr;bNodeSocket *lacunarityFromSock = nullptr;LISTBASE_FOREACH_BACKWARD_MUTABLE (bNodeLink *, link, &ntree->links) {LISTBASE_FOREACH@ -596,0 +712,4 @@if (detailLink != nullptr) {locyoffset -= 40.0f;/* Add Subtract Math node before Detail input. */Would be useful to add a little bit of reasoning for each of these added nodes as comments.
I think the reasoning behind it is too complicated for comments to describe. The PR description is already rather complicated despite only containing hand waving explanations, so I'd rather leave it at just that rather than having some potentially misleading comments
@ -145,6 +221,19 @@ class NoiseFunction : public mf::MultiFunction {const VArray<float> &detail = params.readonly_single_input<float>(param++, "Detail");const VArray<float> &roughness = params.readonly_single_input<float>(param++, "Roughness");const VArray<float> &lacunarity = params.readonly_single_input<float>(param++, "Lacunarity");/* Initialize to any other variable when unused to avoid unnecessary conditionals */End comments with a dot.
After looking at this more closely, I'm not sure about this change anymore.
From the description in the devtalk topic I had understood that the Noise Texture node already contained all the features of the Musgrave node, and agreed with the change because of that. And also assuming it would simplify the code significantly. But it seems that was only for the fbm mode and the Noise Texture is now getting an additional enum and 2 input sockets for the other modes.
For the fbm mode it is indeed redundant, but the other modes the purpose is rather different and was made for a height output that is not in the 0..1 range. Those modes make less sense for a more generic noise texture node that can be used as a building block. So in terms of organization, it's not clear to me it's helpful to merge in those modes mainly meant for modelling terrains.
Additionally, this is going to break forward compatibility which we try to avoid for minor releases like 4.1. It's possible to add some code compensating for that in 4.0 and 3.6, but it's extra code complexity.
I apologize for not checking this sooner.
@brecht
The "additional" enum and 2 were already there, they've just been moved form the Musgrave to the Noise Texture.
And it removes over 2 thousand lines of code which in I'd consider a significant code simplification.
While it's true that historically the Musgrave noise modes were created for modelling terrains that's also the case for the fBM mode. It was only when artists started using it for creating other things that it became generic.
That's also one of my goals for this PR. I don't see why the other Musgrave noise modes should be confined to just creating terrain.In my eyes they're just as usable as generic noise types however the Musgrave Texture node makes using it for anything other than creating terrains very challenging because the Dimension input really only works when creating terrains, which is why I replaced it by the more generic Roughness input.
I understand that for artists used to the Dimension input it will take a little time to get used to the new Roughness input, however I believe without a doubt that the new Roughness input will be easier to use for them in the long term.
When creating procedural noise texture you usually think of the scale of the individual layers that is the Lacunarity input and the height of the individual layers that is the Roughness input as separate things. The Dimension input only controls the height of the layers but needs 2 inputs (both the Dimension and Lacunarity) input to do that. So if you change the Lacunarity value you also change the height of the layers. This kind of entanglement works for creating terrains, but also only terrains, because it was made with creating terrains in mind, but again it prevents it from being used for more generic use cases.
Apart from that I doubt that most artists actually understand what transformations the Dimension input value undergoes before it finally influences the height of the noise layers but rather just tweak it until they get their wanted result.
For that the Roughness input is far easier to use than the Dimension input, as it neither has inter-dependencies nor undergoes an exponentiation.
I could easily add an checkbox, which turns the Roughness input to the Dimension input, but I deliberately decided against it because I don't want artists use an input which they don't really understand. If there turns out to really be demand for it I can still to it in a later PR.
I'm aware that we try to keep forward compatibility for minor releases, which was why I tried to get it in 4.0. We also talked about that in the Nodes & Physics module meeting and as far as I've understood the decision was that we'll take the hit and break forward compatibility for this.
That's more than I thought. Maybe there is some way to do much of this simplification without breaking forward compatibility? Like can we keep the Musgrave Texture node on the user level (perhaps changed to Roughness), but have the implementation be shared? At least for Cycles and Eevee this seems possible.
Right, it's not more complex overall, my concern with the addition of these things is that they're rather specialized for a generic Noise Texture that is more of a building block.
I can imagine that noises like these are useful for use cases besides bump and displacement mapping. But at the moment only fBm has a Normalize option, which I think is important for it to be more generally useful, for use cases like blending weights.
Is there an interpretation of these other modes where they output values in the 0..1 range? You can of course compute min/max bounds somehow, but I'm wondering if they give a somewhat evenly distributed output in the 0..1 range then?
Right, I'm not concerned about this aspect, I agree Roughness is a better parameter.
I really think we should have some kind of forward compatibility, even if this landed in 4.0. Sometimes we have to break it for important reasons, but for me this is not a big enough benefit to justify it. Though removing 2k lines of code is tempting, but maybe most of that is possible without breakage.
@blender-bot package
Package build started. Download here when ready.
My current perspective here is that it's nice to remove unnecessarily duplicated code and to unify inputs to noise nodes where appropriate.
Beyond that, I'm more interested in a good Swap node operator so that switching between different noise types is a unified experience regardless of whether one is switching between noise, musgrave modes, voronoi modes, magic texture, etc.
I do agree that it is hard to justify breaking compatibility here.
It really seems like forward compatibility is the main problem here. In that case I can add code to convert the Noise Texture back into the Musgrave Texture for the 3.6 and 4.0 versions.
For me it's not just forward compatibility, I'm still not convinced that all these modes belong in a single node from a user point of view. I just don't see a compelling reason for it, and no good interpretation of these height fields into a 0..1 range.
Looking at the implementation more closely, it seems like a lot of the code simplification is through templating and macros, and a fair amount of versioning code is added. So probably we can still reduce the code by quite a lot even if we don't merge the nodes. In geometry nodes for example you could share a single
class NoiseFunction : public mf::MultiFunctionbetween the nodes.I also checked if we really need all those distinct modes, if there is perhaps some way to turn some of those into float parameters for a more flexible combined mode. The differences are a lot smaller than you would expect, it's obscured by some loop unrolling and other reshuffling of the code for no good reason. But still not so easy. There are some arbitrary differences like Multifractal not being centered around zero like the other modes.
Based on this, I would not merge the nodes but rather just simplify the implementation.
The only difference between the modes is that they call a different fractalization function, so I don't see why there should be multiple nodes for such a minor difference. I also don't see how that argument goes against merging the nodes. The modes already are in a single Musgrave Texture node so they would still be in a single node even if we don't merge them.
The versioning consists of 2 parts: Firstly it changes the Musgrave node Detail input to match the behavior of the Noise Texture, fixing a bug in the process. Secondly it converts the Dimension values to roughness values.
Even if we don't merge the nodes and rather only simplify the implementation it would still make sense to have these changes, so you'd need the versioning code either way.
Perhaps we could combine some modes together. But I'm certain that it's not possible to combine them all together into a single general mode. So even if, let's say , we could combine all Multifractal modes together, you'd still have 3 distinct modes where the only difference is the fractalization function. Again, I don't see why there should be multiple nodes for such a minor difference they're all just some flavor of Perlin noise. If we have a look at the Voronoi Texture the differences are much greater yet it hasn't been split up either.
Honestly, I don't see how simplifying the implementation would solve any of the problems you addressed above.
I think there is a distinction between a node meant for height fields, centered around 0 without bounds. And a node for textures that produces normalized values in the 0..1 range. It might be a minor mathematical difference, but it's about purpose.
I'm not sure it's worth breaking forward compatibility for that.
I don't think there is a serious problem to be solved in the first place.
It's just nice if we can simplify the code and/or the user interface. So if there is no compelling reason to change the user interface and deal with the associated breakage, it would be nice to still simplify the code.
When Normalize is turned off the current Noise node already produces values outside the 0.0 -1.0 range. The same holds true for the Voronoi Texture. I also don't understand why you think we should be confined to a 0.0 -1.0 range when creating textures. Sure the final texture should be in a 0.0 -1.0 range to keep energy preservation but it's very common to have values outside that range somewhere in the node tree.
I add some versioning code to the 3.6-release and 4.0-release branches and voilà we have forward compatibility.
The Musgrave Texture has modes that the Noise Texture doesn't. Meanwhile the Noise Texture has a Distortion input and Color output which the Musgrave Texture doesn't. It's impossible to use both features simultaneously. So merging the nodes does effectively add features.
@brecht Looks like the artists on https://devtalk.blender.org/t/merging-the-musgrave-texture-and-noise-texture-nodes/30646 are happy now after resolving some communication mistakes.
There are even already example images of the new features I mentioned: https://devtalk.blender.org/t/merging-the-musgrave-texture-and-noise-texture-nodes/30646/67?u=hoshinova
@blender-bot package
Package build started. Download here when ready.
Ok, let's do it then. Python API breaking changes are inevitable. But I guess along with Gabor noise this will be a nice improvement to texturing, and there are various other breaking changes in 4.1 as well.
If it's possible to have versioning code for 4.0 by next week Wednesday, that would be great so we can add it in time for 4.0.0.
@brecht
I'm very busy right now but I'll try my best. For the 4.0 versioning I just work on the
blender-v.4.0-releasebranch right?As for the new features which don't exist in 4.0, like the Distortion input and Color output for the Musgrave modes, should I make the versioning code add nodes that emulate the behavior or should I just ignore these inputs/outputs?
Thanks!
Yes.
You can ignore those.
@brecht
@ -1272,3 +1272,4 @@NodeTexBase base;int dimensions;uint8_t type;uint8_t normalize;Will Blender 4.1 read
uint8_t normalizecorrectly? Because the memory space that used to be foruint8_t normalizein 4.0 is now used foruint8_t type.Will Blender 4.1 interpret
uint8_t normalizeasuint8_t type?@blender-bot package
Package build started. Download here when ready.
@ -20,3 +8,1 @@maxamp += amp;amp *= clamp(roughness, 0.0, 1.0);fscale *= lacunarity;#define NOISE_FBM(T) \Judging by
ea2746d468. It seems we can't use such a macro.Unroll and add a TODO comment like Jeroen did. Same goes for the glsl file below, since using a macro for the body also didn't work.
@OmarEmaraDev Not necessarily. In case of
ea2746d468it was onlyFRACTAL_VORONOI_X_FX(T)that was failing, whileFRACTAL_VORONOI_DISTANCE_TO_EDGE_FUNCTION(T)works correctly and is still a macro inmain. I suggest someone who has access to a legacy Intel platform test it and we only unroll the functions that are failing, if any.@Jeroen-Bakker Could you do some testing?
Macro unrolling might fail on specific laptops where user can only use the driver from the laptop manufacturer. I don't have access to such a laptop so I am not able to test.
I would just continue with the macros and if there are issues we should manually unroll them, until we do our own GLSL compilation or transit to vulkan.
@blender-bot package
Package build started. Download here when ready.
@blender-bot package
Package build started. Download here when ready.