Paths

Table of Contentst

Fix T102210: Clicking on the Camera icon of the jobs panel should focus Render Window during rendering
Needs ReviewPublic
Actions

Authored by Alexander Hartmann (Alex.H) on Nov 2 2022, 7:12 PM.

Details

Reviewers

Jeroen Bakker (jbakker)
Julian Eisel (Severin)

Maniphest Tasks

T102210: Render: During rendering the clicking on the Camera icon of the jobs panel, should focus/open a Render Window.

Summary

'job icon' and 'job name' are now separated from each other.
'job icon' is converted to a button when a 'render' job is running.

Diff Detail

Repository: rB Blender

Event Timeline

Alexander Hartmann (Alex.H) requested review of this revision.Nov 2 2022, 7:12 PM

Alexander Hartmann (Alex.H) created this revision.

Alexander Hartmann (Alex.H) edited the summary of this revision. (Show Details)Nov 2 2022, 7:16 PM

Alexander Hartmann (Alex.H) mentioned this in T102210: Render: During rendering the clicking on the Camera icon of the jobs panel, should focus/open a Render Window..

make format

Pratik Borhade (PratikPB2123) retitled this revision from T102210 to Fix T102210: Clicking on the Camera icon of the jobs panel should focus Render Window during rendering.Nov 3 2022, 3:36 AM

Pratik Borhade (PratikPB2123) edited the summary of this revision. (Show Details)

Pratik Borhade (PratikPB2123) awarded a token.

Pratik Borhade (PratikPB2123) added a task: T102210: Render: During rendering the clicking on the Camera icon of the jobs panel, should focus/open a Render Window..

Pratik Borhade (PratikPB2123) edited reviewers, added: Jeroen Bakker (jbakker); removed: User Interface.

Thanks for picking this up!

Sorry to point you directly in a legacy part of blender. Other areas are more readable than this part. :-)

Before assigning it to the correct module I would like to test using the uiDefIconButO. IMO it would make the code more clearer as it doesn't break the flow when reading code. It would also make the change smaller. After that we can check with the UI module to accept this patch.

The title + description of this patch will be used as commit message. Therefore it should follow the guidelines of a commit message. https://wiki.blender.org/wiki/Style_Guide/Commit_Messages

source/blender/editors/interface/interface_templates.c
5969	When looking at this comment in a few months/years this doesn't have a meaning. It is better to add these in the patch description. So tools can still track them down. We normally don't refer to ticket numbers for new features. Only for exceptions, work-arounds etc. Key bindings are dynamic so perhaps not mention them.
6168	In stead of adding more complexity to other areas in the code we could switch `uiDefIconButO` based on the operator that could be assigned by the switch statement. I am not 100% as I normally don't touch this area of the code, but could you try to get this working? This might remove the need for the `B_VIEWRENDER`.

This revision now requires changes to proceed.Nov 8 2022, 8:36 AM

Thank you for the hints!
I'll try to revise it today or tomorrow.

Now less changes. The button is created via uiDefIconButO. The tooltip text is inserted by the function.

WM_JOB_TYPE_RENDER

Default UI type is a label when other jobs are running.

Alexander Hartmann (Alex.H) edited the summary of this revision. (Show Details)Nov 9 2022, 6:27 PM

In D16372#446807, @Jeroen Bakker (jbakker) wrote:

The title + description of this patch will be used as commit message. Therefore it should follow the guidelines of a commit message. https://wiki.blender.org/wiki/Style_Guide/Commit_Messages

I am not sure yet to follow the 'Style Guide' correctly. I have also checked other accepted 'differentials': https://developer.blender.org/differential/

The title was adjusted by 'Pratik' a week ago.
For me it's more of a UI improvement and not a bug fix. The guide says:

The first line represents the subject of the commit.
The body the commit messages must be separated from the subject by a blank line.

Is it still necessary if the title is already used?

Austin Berenyi (sooperturkey) awarded a token.Nov 14 2022, 8:31 PM

Austin Berenyi (sooperturkey) added a subscriber: Austin Berenyi (sooperturkey).

Unclear to me, but it doesn't seem to work on my end. Does it work for you?

About the title of the patch. It is not required, but adding Fix T number the patch gets automatically closed when commit.
For this patch I wouldn't use the Fix prerequisite as it is already linked to that ticket.

source/blender/editors/interface/interface_templates.c
6048	Can use `eButType` in stead of `int`.
6049	Can be a `const char * = NULL` this way you don't need to copy the operator.

job_icon_ui_type is now eButType
job_icon_opname is now const char *

In D16372#449499, @Jeroen Bakker (jbakker) wrote:

Unclear to me, but it doesn't seem to work on my end. Does it work for you?

I have used 'master branch' (Date: Tue Nov 15 18:41:24 2022 +0100)
and adjusted the data types, so far no problems detected.

@Jeroen Bakker (jbakker) and @Julian Eisel (Severin)
Should I remove my assignment if my solution is not good enough? So that someone else can try.

I ran 'merge' today and found no conflicts. It still works as in the video. But I had to delete build folder for xcode under MacOs again. A few days ago I updated to MacOs 13.1.

    Merge branch 'master' into T102210-clicking-on-the-Camera-icon-Render-Window

commit 28511ac6cf83dfd56026125ff97630becb44fdf2 (master)
Author: Jacques Lucke <jacques@blender.org>
Date:   Sat Dec 17 14:46:15 2022 +0100

Would love to see this feature work out!

Revision Contents
Changeset List

Path

Size

source/

blender/

editors/

interface/

interface_templates.c

54 lines

Diff 57208

View Options

source/blender/editors/interface/interface_templates.c

Fix T102210: Clicking on the Camera icon of the jobs panel should focus Render Window during renderingNeeds ReviewPublicActions