GPv3: Fill texture coordinates data storage #114772
No reviewers
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
4 Participants
Notifications
Due Date
No due date set.
Depends on
#115783 BLI: Add support for non-square matrix multiplication.
blender/blender
#116077 BLI: Add diagonal matrix from vector
blender/blender
Reference: blender/blender#114772
Loading…
Reference in New Issue
No description provided.
Delete Branch "casey-bianco-davis/blender:GPv3-fill-texture"
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 is proposes a new system for grease pencil v3 fill texture coordinates.
In the legacy system:
Transform Fill
operator which was extremely unintuitive and hard to use.This pull request adds:
This system could allows for future features such as:
GPv3: New fill texture coordinatesto GPv3: Fill texture coordinates systemSo, that is just 2d position projection into surface of curve?
Yes, this system should look the same as the legacy system only differing in how the data is stored and accessed.
Thanks for working on this 🙂 Just have a comment about the change to
BLI
for now.@ -384,1 +382,4 @@
/** Multiply two compatible matrices using matrix multiplication. */
template<int OtherNumRow>
MatBase<T, OtherNumRow, NumRow> operator*(const MatBase<T, OtherNumRow, NumCol> &b) const
Generally, changes to
BLI
should be in their own PR, preferably with a new test added toblender/source/blender/blenlib/tests/BLI_math_matrix_test.cc
😉GPv3: Fill texture coordinates systemto GPv3: Fill texture coordinates data storagecasey-bianco-davis referenced this pull request2023-12-11 22:51:09 +01:00
GPv3: Fill texture coordinates data storageto WIP: GPv3: Fill texture coordinates data storageWIP: GPv3: Fill texture coordinates data storageto GPv3: Fill texture coordinates data storage@filedescriptor All dependencies are done, this PR is ready to be reviewed.
I think we should change the attributes to store a loc/rot/scale transform instead of the barycentric vectors + offset. I don't want to expose the legacy 3-point method unless necessary. And if the texture transform is already stored as general barycentric unit vectors then we might as well store the linear transform directly.
@ -868,0 +870,4 @@
* `curve_i` Note: the matrix returned does not represent an actual transformation and instead just
* stores the three points.
*/
blender::float3x3 get_texture_points(const blender::bke::CurvesGeometry &curves, int curve_i);
This function is used only inside grease_pencil.cc, doesn't need to be exposed in the header.
Storing the points in a
float3x3
is confusing, i would suggest returning e.g.float3[3]
. Better yet, since these points are only used to compute an (orthonormal) transform the function could just do that and return a float4x4.I had to implement the same function for the texture modifier. It won't be needed there anymore when this change goes in since the modifier would just change the linear transform.
2c9e34e168/source/blender/modifiers/intern/MOD_grease_pencil_texture.cc (L82-L101)
Currently
get_texture_points
is only used in the same file, but it will be used in a future PR.I agree with not using
float3x3
but I preferstd::array<float3, 3>
over c-style arrays.@ -1457,0 +1496,4 @@
MutableAttributeAccessor attributes = curves.attributes_for_write();
SpanAttributeWriter<float3> textU = attributes.lookup_or_add_for_write_span<float3>(
I suggest to just store the loc/rot/scale instead of three vectors. That's a bit more flexible and doesn't rely so much on this arbitrary GPv2 method of picking three points to generate the transform.
The pair of
get_texture_points
/set_texture_points
would then be replaced byget_stroke_plane_transform
/set_stroke_plane_transform
.@ -159,0 +171,4 @@
/* Center of rotation. */
const float2 center = float2(0.5f, 0.5f);
const float2 uv_scale_inv = float2(uv_scale.x != 0.0f ? 1.0f / uv_scale.x : 1.0f,
math::safe_rcp
does 1/x with a null check@ -159,0 +188,4 @@
textmat[2] += uv_translation;
/* Apply rotation. */
textmat[2] -= center;
Could use
math::from_origin_transform
for thismath::from_origin_transform
doesn't work here becausetextmat
is float3x2 androt
is float2x2 andmath::from_origin_transform
only works with same size matrices.@ -569,3 +570,2 @@
s_vert.u_stroke = length;
/* TODO: Populate fill UVs. */
s_vert.uv_fill[0] = s_vert.uv_fill[1] = 0;
copy_v2_v2(s_vert.uv_fill, texture_matrix * float4(pos, 1.0f));
For applying a transform to a point use
math::transform_point(texture_matrix, pos)
math::transform_point
does not work here because the texture matrix is 4x2, projecting points from 3D local-space to 2D texture-space. Whereasmath::transform_point
only works with 3D to 3D transformations.Ah, that's alright then, thanks for explaining.
@ -177,1 +179,4 @@
/* TODO: Align with the view or drawing plane. */
/* The default is the front draw plane (XZ). */
texture_points_ = float3x3(
Is this going to be part of the patch or is it not possible atm for some reason?
I was planing on adding more options and UI as a separate PR, so that this PR can be focused only on the storing, displaying and converting of the texture coordinates.
But I will add simple aligning with the drawing plane.
I think that there has been a misunderstanding about what this PR does and how the texture coordinates are stored.
This PR creates three curve attributes, points U, V and Origin (accessed with
texture_u
,texture_v
andtexture_origin
) which are points in local-space that represents the UV coordinates (1,0), (0,1) and (0,0) respectively.These three points are not the same as the "legacy 3-point method".
This PR also removes the attributes
fill_translation
,fill_rotation
andfill_scale
.The three texture points (U, V, Origin) completely determent the texture coordinates.
So before what was store was 2 floats(
fill_translation
) + 1 float(fill_rotation
) + 2 floats(fill_scale
) (5 total floats), this transformation was with respect to the stroke in a very arbitrary and unintuitive way.Whereas here we store 3 floats(
texture_u
) + 3 floats (texture_u
) + 3 floats (texture_origin
) (9 total floats), this transformation is completely regular and deterministic.Note: some of the data stored in each point is redundant, as the point should be on the stroke's plane but can be stored not on the plane.
Why the data is stored this way is so that any operator that changes any point positions will automatically maintain the texture coordinates, (for instance deleting the last point in the legacy system would mess up the whole texture)
Another reason to store the data as points is to make working with
Geometry Nodes
as easy as possible (see the attached image)I think that the eases of use with
Geometry Nodes
makes the redundant data worth it.There was a meeting talking about the texture coordinate system and a different method was decided on. PR #119303
Pull request closed