Add option to preserve UV:s in ```Entities.transform_by_vectors```

[CAUL2000]

Entities.transform_by_vector is currently a very fast and convenient way to manipulate huge amounts of geometry. However, this is countered by the fact that the method destroys all UV's. In practice, what has to be done is something like:

Back all UV's in ruby # slow
Entities.transform_by_vectors(sub_entities, vectors) #extremely fast
Restore all UV's in ruby # slow

The supporting UV-handling in ruby makes a very fast method rather slow. It would be a welcome addition to the method if it could be made to optionally preserve UV's. Something like:

Entities.transform_by_vectors(sub_entities, vectors, preserve_uv = true)

This enhancement would serve the same purpose as the speed improvements to Polygon Mesh, Explode and Entities Builder.

[Eneroth3]

Thanks for logging this!

[thomthom]

Can you supply a sample that demonstrates the performance? With performance related issues it's very useful to have exact code examples and data to measure against.

[DanRathbun]

I would like to ask why would the preservation of UVs need to be optional ? In what scenario(s) would extension code need the current behavior that destroys the UV mapping?

Ie, ... it is likely problematic to add a new boolean argument onto the end of an already established method. Older API versions would not be expecting the extra argument and raise an ArgumentError. Going forward this would force you to always use the following pattern:

entities.transform_by_vectors(vertices, vectors, true) rescue entities.transform_by_vectors(vertices, vectors)

... or ...

begin
  entities.transform_by_vectors(vertices, vectors, true)
rescue ArgumentError
  backup_uvs(entities)
  entities.transform_by_vectors(vertices, vectors)
  restore_uvs(entities)
end

Note that some coding style guides frown upon using the rescue clause as a branching statement like this and instead encourage something like so ...

if entities.method(:transform_by_vectors).arity > 2
  entities.transform_by_vectors(vertices, vectors, true)
else
  backup_uvs(entities)
  entities.transform_by_vectors(vertices, vectors)
  restore_uvs(entities)
end

But we cannot do this ("duck-typing") as Method#arity always returns -1 for methods written in C.

To use an if conditional you would need to use a Sketchup.version test.

So the alternatives are:

• Implement a new method
• Fix the current method

[DanRathbun]

Okay? Since it seems that a Sketchup.version would likely be needed if supporting older API versions anyway, (because "duck-typing" whether UV preservation is implemented is not possible,) ...

... I would argue that the choice should favor fixing the current method, and then if a programmer must provide the backup and restore UV pattern that they can do this in the else clause.

if Sketchup.version.to_i >= UVS_PRESERVED
  entities.transform_by_vectors(vertices, vectors)
else # UVs are not preserved, so backup & restore:
  backup_uvs(entities)
  entities.transform_by_vectors(vertices, vectors)
  restore_uvs(entities)
end

[Eneroth3]

I think there could be use cases for the current behavior. I think you (nearly) always want to "keep" the UV mapping, but what "keep" means can differ. If you have a terrain mesh where you set UV per vertex, you want to keep the UV at those vertices when moving them around. If you draw a cabinet and position the material in the SU GUI on the faces, not the vertices, you don't want your wood texture stretched just because you make the cabinet wider.

I'm thinking a parameter is the safest and cleanest way to have it both ways and not risk breaking anything.

[CAUL2000]

Here's a model and a code example as a zip. The snippet has a parameter in the main method controlling whether to preserve the uvs or not. transform_by_vectors_test.zip

[thomthom]

But we cannot do this ("duck-typing") as Method#arity always returns -1 for methods written in C.

That depends how it's implemented. If the C code define the method with variable number of arguments;

# C variant
rb_define_method(klass, "my_method", my_method,  -1);

# Ruby variant
def my_method(*args)
end

But defined with a fixed number of arguments, arity with yield a positive number.

# C variant
rb_define_method(klass, "my_method", my_method, 2);

# Ruby variant
def my_method(arg1, arg2)
end

In the case of transform_by_vectors it actually accept variable length params, anything after the first (transform) get treated as a list of entities to transform. It's not documented and there's probably more like this. I've been documenting it as I've come across it. This very flexible overload style is somewhat of a pain to mantain and document and I wish it wasn't so prevalent.

Any additional arguments would have to be named arguments. Which in this case, would make sense as it would be an option;

entities.transform_by_vectors(sub_entities, vectors, preserve_u: true)

I would like to ask why would the preservation of UVs need to be optional ? In what scenario(s) would extension code need the current behavior that destroys the UV mapping?

If you want to have the same behaviour as the UI tools. There are arguments for having UVs pinnable to vertices in the UI as well and I think that would be the true ideal change related to this. But even then I see usages for this being optional. There are many times I wanted the texture to tile as-is without being stretched by moving vertices around. For instance when changing the length of a wall with a brick texture - don't want the bricks to squash.

[DanRathbun]

@thomthom : _In the case of transform_by_vectors it actually accept variable length params, anything after the first (transform) get treated as a list of entities to transform. It's not documented and there's probably more like this. I've been documenting it as I've come across it._ ...

I'm sorry but I think you are confounding separate methods. What you are describing is the transform_entities() method.

The transform_by_vectors method must have two equal size arrays where the number of entities (first argument) matches the number of vectors (the second argument). The method documentation implies that this uses translational vectors, but can it also take any kind of transformations in the 2nd array ?

@thomthom : Any additional arguments would have to be named arguments. Which in this case, would make sense as it would be an option;

I guess I don't much care in this case as code would have to use a version sniffing conditional expression regardless.

But I feel that the default would most likely should be to preserve UVs rather than not. Since it's a breaking change and would need to be decided by version sniffing then why not have the default be what would be most used ?

[thomthom]

Since it's a breaking change and would need to be decided by version sniffing then why not have the default be what would be most used ?

That's a big assumption. By experience, in maintaining an API with the aims for it to be stable it's best to preserve behaviour as far as possible unless it's needed to correct corruption or crashes. Even minor changes we thought would have no impact has proven us wrong. Once something has been released it warrant a completely different set of considerations than if creating something new.

[DanRathbun]

That's a big assumption.

Then take a poll of those using the method.

[thomthom]

Then take a poll of those using the method.

That's also assuming we have the ability to identify all users of the API - which we don't.

This comes back to fundamental API design best practices; you don't change the behaviour of released features unless you absolutely have to. Stability is alpha and omega.

[Eneroth3]

We are not taking about a bugfix here but an enhancement. The proposed functionality could be very useful in various cases, but there are also cases where the existing behavior is valid. You may want to widen a cabinet or a extend a plank or do other changes, without stretching a tiled texture. The new behavior would no doubt be useful, but not in every possible existing valid use case, which is why it cannot be the default.

We have had this discussion many times before and should not have to have it again.

[DanRathbun]

Well the discussions are not collected together in some sort of central searchable dictionary, so it is quite difficult for everyone to "be on the same page". IE ... there is no easily navigated public knowledge base for the API.

[Matherone]

Returning to the topic:

Yes, please do.

For our applications, if the API supported a parameter that preserves UV:s while transforming by vector, this would create a lot of value indeed.

In certain common scenarios, such as working with large draped textures, an option to preserve UV:s would add a massive performance improvement.

It would enable us and other developers to help end users who are reporting that they have struggled with this for a long time. There are further use cases as well. There are ways of adding this option while keeping legacy behaviour.

Please add option to preserve UV:s in Entities.transform_by_vectors

[thomthom]

In certain common scenarios, such as working with large draped textures, an option to preserve UV:s would add a massive performance improvement.

Can you share an example model and snippet to demonstrate this performance issue. When dealing with performance issue it's important that we have good real world examples to go by. (Remember, just because the API layer would do what you do in Ruby doesn't mean it'll be "free" in terms of performance)

[Oreguru]

Please, add the ability to save texture position when moving points or edges. For example, I import a character model with existing UV coordinates, changing the geometry, the position of the textures collapse and I have to edit it manually using the plugin - Marginal UV Editor v0.3. I can take screenshots if needed. Thank you very much in advance!

[Matherone]

Can you share an example model and snippet to demonstrate this performance issue. When dealing with performance issue it's important that we have good real world examples to go by. (Remember, just because the API layer would do what you do in Ruby doesn't mean it'll be "free" in terms of performance)

Yes. For a clear example model and a snippet that demonstrates the difference in output performance, please see @CAUL2000 's example in his March 3rd post in this thread, above:

Here's a model and a code example as a zip. The snippet has a parameter in the main method controlling whether to preserve the uvs or not. transform_by_vectors_test.zip

The example is clear and simple. In the Ruby file, try changing the argument to "true" or "false", and see for yourself. In this example, our tests yield a consistent performance difference of 5 to 10 times faster.

See one illustration image, below.

• Would you please look into adding a Preserve UV option to this part of the API?