Render: include info string for "render_stats" handler #119789

Open

Philipp Oeser wants to merge 1 commits from lichtwerk/blender:render_stats_handler_include_info_string into main

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

Philipp Oeser commented

2024-03-22 15:57:11 +01:00

Member

The "render_stats" handler now includes the message line that was
written to stdout as the first arg (using BKE_callback_exec_string
instead of just BKE_callback_exec_null).

This can be useful to track render progress (and people were relying on
parsing stdout in the past).

Was a request in chat.

This also corrects possible output from the callback being written in
between message parts in do_write_image_or_movie (which was probably a
mistake in 93d5e106aa).

The "render_stats" handler now includes the message line that was written to stdout as the first arg (using `BKE_callback_exec_string` instead of just `BKE_callback_exec_null`). This can be useful to track render progress (and people were relying on parsing stdout in the past). Was a request in chat. This also corrects possible output from the callback being written in between message parts in `do_write_image_or_movie` (which was probably a mistake in 93d5e106aac8).

Philipp Oeser added 1 commit 2024-03-22 15:57:20 +01:00

84fd9379b9 Render: include info string for "render_stats" handler

The "render_stats" handler now includes the message line that was
written to stdout as the first arg (using `BKE_callback_exec_string`
instead of just `BKE_callback_exec_null`).

This can be useful to track render progress (and people were relying on
parsing stdout in the past).

Was a request in chat.

This also corrects possible output from the callback being written in
between message parts in `do_write_image_or_movie` (which was probably a
mistake in 93d5e106aa).

Philipp Oeser added this to the Render & Cycles project 2024-03-22 15:57:32 +01:00

Philipp Oeser added the

Render Pipeline

label 2024-03-22 15:57:40 +01:00

Philipp Oeser requested review from Brecht Van Lommel 2024-03-22 15:57:47 +01:00

Philipp Oeser requested review from Sergey Sharybin 2024-03-22 15:57:53 +01:00

Brecht Van Lommel commented

2024-03-22 16:00:10 +01:00

Owner

Is this an API breaking change?

Is this an API breaking change?

Philipp Oeser commented

2024-03-22 16:25:51 +01:00

Author

Member

A render_stats handler callback would require 2 positional arguments prior to this change as well, these were always None though, now the first is the message string.
Was a bit surprised by that tbh. (was expecting BKE_callback_exec_null to provide no args at all, but would have to dig a bit deeper how the callbacks are actually called...)

Does this answer the question already?

A `render_stats` handler callback would require 2 positional arguments prior to this change as well, these were always None though, now the first is the message string. Was a bit surprised by that tbh. (was expecting `BKE_callback_exec_null` to provide no args at all, but would have to dig a bit deeper how the callbacks are actually called...) Does this answer the question already?

Sergey Sharybin requested changes 2024-03-22 17:01:43 +01:00

Sergey Sharybin left a comment

Owner

message = BLI_sprintfN("%s (Saving: %s)", message, filepath); is leaking memory, as the previously dynamically allocated message is not freed.

It might be easier to use std::string and fmt::format for these purposes.

`message = BLI_sprintfN("%s (Saving: %s)", message, filepath);` is leaking memory, as the previously dynamically allocated `message` is not freed. It might be easier to use `std::string` and `fmt::format` for these purposes.

Brecht Van Lommel requested changes 2024-03-25 14:54:23 +01:00

Brecht Van Lommel left a comment

Owner

The functionality is fine with me then.

bpy_app_handlers.cc will need to be updated to document this new argument (see FILEPATH_SAVE_ARG for an example).

The functionality is fine with me then. `bpy_app_handlers.cc` will need to be updated to document this new argument (see `FILEPATH_SAVE_ARG` for an example).

Merge conflict checking is in progress. Try again in few moments.

View command line instructions.

Checkout

From your project repository, check out a new branch and test the changes.

git fetch -u render_stats_handler_include_info_string:lichtwerk-render_stats_handler_include_info_string

git checkout lichtwerk-render_stats_handler_include_info_string

Sign in to join this conversation.

No reviewers

No Label

Animation & Rigging

Asset Browser Project Overview

Automated Testing

Blender Asset Bundle

Dependency Graph

Development Management

EEVEE & Viewport

Images & Movies

Motion Tracking

Nodes & Physics

Pipeline, Assets & IO

Platforms, Builds & Tests

Render & Cycles

Render Pipeline

Sculpt, Paint & Texture

Video Sequencer

Virtual Reality

Blender 2.8 Project

Milestone 1: Basic, Local Asset Browser

Good First Issue

Animation & Rigging

Development Management

EEVEE & Viewport

Nodes & Physics

Pipeline, Assets & IO

Platforms, Builds & Tests

Render & Cycles

Sculpt, Paint & Texture

Needs Info from Developers

Needs Information from User

No Milestone

No project

No Assignees

3 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#119789

No description provided.