WIP: Fix #53574: Rename "Degr" socket of the Rotate compositing node to "Angle" #110637

Draft
Damien Picard wants to merge 1 commits from pioverfour/blender:dp_rename_degr_angle into main

When changing the target branch, be careful to rebase the branch in your fork to match. See documentation.
Member

The "Degr" socket of the Rotate compositing node is used to store the
angle of rotation. It is needlessly shortened, but it is also wrong, as
the unit depends on the scene settings and can also be radians.

Similarly to !108234, this commit renames it to "Angle", with a
versioning for existing nodes.

Ref #96219


Before After
image image
The "Degr" socket of the Rotate compositing node is used to store the angle of rotation. It is needlessly shortened, but it is also wrong, as the unit depends on the scene settings and can also be radians. Similarly to !108234, this commit renames it to "Angle", with a versioning for existing nodes. Ref #96219 ----- | Before | After | |-|-| | ![image](/attachments/f1873243-67cd-43cb-a287-5ef81b936138) | ![image](/attachments/a750d389-64c7-4629-af01-876d05baa4a2) |
Damien Picard added 1 commit 2023-07-31 00:08:33 +02:00
The "Degr" socket of the Rotate compositing node is used to store the
angle of rotation. It is needlessly shortened, but it is also wrong, as
the unit depends on the scene settings and can also be radians.

Similarly to !108234, this commit renames it to "Angle", with a
versioning for existing nodes.
Damien Picard added this to the Compositing project 2023-07-31 00:08:42 +02:00

UI: Composting nodes Rotate socket rename #53574

UI: Composting nodes Rotate socket rename #53574
Author
Member

UI: Composting nodes Rotate socket rename #53574

Oh I didn’t see this one, thanks. There was a PR for it a few months ago, but I can’t see its contents.

> UI: Composting nodes Rotate socket rename #53574 Oh I didn’t see this one, thanks. There was a PR for it a few months ago, but I can’t see its contents.
Damien Picard changed title from UI: Rename "Degr" socket of the Rotate compositing node to "Angle" to Fix #53574: Rename "Degr" socket of the Rotate compositing node to "Angle" 2023-07-31 16:22:52 +02:00
Pratik Borhade requested review from Brecht Van Lommel 2023-08-04 10:58:43 +02:00

Note that any socket renaming is still blocked on a solution to the identifier/name problem in the Python API. I do not plan to accept any renaming patches until that, and it's unclear if there will be time for it before 4.0.

Note that any socket renaming is still blocked on a solution to the identifier/name problem in the Python API. I do not plan to accept any renaming patches until that, and it's unclear if there will be time for it before 4.0.
Author
Member

@brecht Should !108234 be reverted then?

@brecht Should !108234 be reverted then?
Brecht Van Lommel refused to review 2023-08-04 17:39:03 +02:00

Breaking API compatibility for these compositing nodes is not as bad as for shader nodes, so I think that could stay. In that context maybe this change is also ok. But for shader nodes at least there are various exporters relying on socket names, so there it is blocked.

But I'm not a maintainer of the compositor, so not really the appropriate person to review.

Breaking API compatibility for these compositing nodes is not as bad as for shader nodes, so I think that could stay. In that context maybe this change is also ok. But for shader nodes at least there are various exporters relying on socket names, so there it is blocked. But I'm not a maintainer of the compositor, so not really the appropriate person to review.

We did accept some renames in the compositor recently, but it was also followed with the discussion of compatibility.

I do find it important to keep compatibility with exporters, and I don't really know why we should treat different node systems differently.

We had similar discussion (in the chat) about the previous rename, and we decided all things considered it was a low risk change. Mainly because the node was unlikely to be used (the motion tracking one is much more commonly used, as it has better interop with actual calibration data). The rotation node is probably more commonly used by scripts.

I didn't mind doing the risk assessment once, but doing it on a regular basis is rather time and energy consuming. Think it would be easier for everyone if we could delay this rename until python identifier and name are decoupled.

We did accept some renames in the compositor recently, but it was also followed with the discussion of compatibility. I do find it important to keep compatibility with exporters, and I don't really know why we should treat different node systems differently. We had similar discussion (in the chat) about the previous rename, and we decided all things considered it was a low risk change. Mainly because the node was unlikely to be used (the motion tracking one is much more commonly used, as it has better interop with actual calibration data). The rotation node is probably more commonly used by scripts. I didn't mind doing the risk assessment once, but doing it on a regular basis is rather time and energy consuming. Think it would be easier for everyone if we could delay this rename until python identifier and name are decoupled.
Author
Member

That’s fine, I’ll mark this as WIP: in the mean time. Thanks.

That’s fine, I’ll mark this as WIP: in the mean time. Thanks.
Damien Picard changed title from Fix #53574: Rename "Degr" socket of the Rotate compositing node to "Angle" to WIP: Fix #53574: Rename "Degr" socket of the Rotate compositing node to "Angle" 2023-08-11 16:42:27 +02:00
This pull request has changes conflicting with the target branch.
  • source/blender/blenloader/intern/versioning_300.cc
  • source/blender/nodes/composite/nodes/node_composite_rotate.cc

Checkout

From your project repository, check out a new branch and test the changes.
git fetch -u dp_rename_degr_angle:pioverfour-dp_rename_degr_angle
git checkout pioverfour-dp_rename_degr_angle
Sign in to join this conversation.
No reviewers
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
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
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
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
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: blender/blender#110637
No description provided.