Page MenuHome

Convert printfs to CLOG
Needs ReviewPublic

Authored by Mateusz Grzeliński (grzelins) on Aug 24 2020, 1:05 PM.

Details

Summary

Brief: Diff against master part 2 - add printf to clog, depends on changes in part 1: D8677: Add core functionalities from GSoC Info Editor Improvements

  • reduce use of G_DEBUG guards
  • reduce use of #ifdef guaraded printfs
  • convert print functions to sprintfN style
  • reduce commented printfs
  • avoid making prints dependent on G.background

Highlights:

  • log every 30-th render stat update (external_engine.c:504, related: eevee_render.c:545), render engine chooses when to call update: in eevee per sample, in cycles every second
    • pros:
      • avoid spamming multiple render stats,
      • render stats can be actually enabled or disabled,
      • we can implement json like log format (currently not implemented),
      • avoid slowing down windows console (see eevee_render.c:545)
    • cons: important messages like start, cancel can be missed in current implementation

I described my thought process in this post

Diff Detail

Repository
rB Blender
Branch
D8691-soc-submit-convert-to-clog (branched from master)
Build Status
Buildable 9773
Build 9773: arc lint + arc unit

Event Timeline

Mateusz Grzeliński (grzelins) requested review of this revision.Aug 24 2020, 1:05 PM
Mateusz Grzeliński (grzelins) created this revision.

log every n-th render stat

It's unclear what this means exactly. But note that for render farms, we need full logs, you can't skip printing part of the stats. We should not change the format unless there is a major improvement, since this will break scripts that parse this output.

source/blender/blenfont/BLF_api.h
36

This should not be in the public API.

On a more general note, there is a distinction between log messages that are interesting to users and to developers.

For messages for users, we should not print anything like source file location or line numbers, and any important warnings or errors should be printed by default without the user having to enable logging.

On a more general note, there is a distinction between log messages that are interesting to users and to developers.

Oh, I assumed all logs are mean for developers and reports are meant for users. But it will be easy to implement as log records can hold flags (which then can be filtered)

  • Remove log from public API

Oh, I assumed all logs are mean for developers and reports are meant for users. But it will be easy to implement as log records can hold flags (which then can be filtered)

I think it is correct that so far logs were meant for developers only. Which is why I don't understand why render stats and errors were changed to use the logging system. I only checked the render module, there may be messages in other modules that are also meant for (technical) users.

In any case, it is not up to the logging system to decide which subset of render stats to show. There could be verbosity levels in render stats and we could default to less verbose stats by default, but using every Nth stats is not a solution I would accept.

I don't have a strong opinion if the logging system or reports system should be extended. But the distinction needs to be there in the API, and user and developer messages need to be handled differently.

Besides users vs. developer, it's also important for most user messages can happen within a specific context. Operator reports are associated with a particular operator execution, render stats should be associated with a specific render, depsgraph stats should be associated with a specific scene and view layer. With multithreading multiple tasks can be executed at the same time, and we don't want such messages to be mixed. The reports systems can handle this since it explicitly passes on a ReportList, instead of using globals.