Skip to content

Conversation

sbrood
Copy link
Contributor

@sbrood sbrood commented Jan 11, 2023

Description

Improvement of computation information in node editor header

Features list

[X] update computation time every 2.5 seconds in header
[X] computation time when error occurs
[X] chunk computed counter in node editor header

Implementation remarks

The previous label text logic is moved to the nodeChanged signal because the logic is overwritten by the timer when setting the text time.
I don't know if 2.5 seconds is the best choice, feel free to give other refresh timing ideas.

@sbrood sbrood self-assigned this Jan 12, 2023
@fabiencastan fabiencastan added this to the Meshroom 2023.1.0 milestone Jan 14, 2023
Copy link
Member

@fabiencastan fabiencastan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

need some modifications

@sbrood sbrood requested a review from fabiencastan January 16, 2023 11:22
@sbrood sbrood requested a review from fabiencastan January 18, 2023 16:21
Copy link
Member

@fabiencastan fabiencastan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we change the selected node and the status is the same, the information is not updated (and we display the timing info of another node).

@fabiencastan fabiencastan marked this pull request as draft January 22, 2023 16:26
@sbrood sbrood requested a review from fabiencastan January 23, 2023 08:24
@sbrood sbrood marked this pull request as ready for review January 23, 2023 09:08
Copy link
Contributor

@cbentejac cbentejac left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you select a node, start its computations, and wait without unselecting it, the timer never stops if the status is updated to ERROR: the displayed computation time keeps on increasing as the timer keeps on being triggered.
If you unselect the node, the timer is immediately stopped, and if you reselect it, the displayed computation time is correct.

I don't know where it happens, but it seems like the globalStatusChanged signal is not emitted when the node's status changes to ERROR.

On another note, since we compute the elapsedTime when we stop the computations (cf the except clauses in node.py), couldn't we display it in that case as well? (it is an open question)

@Just-Kiel Just-Kiel force-pushed the dev/nodeComputationDisplay branch from 3c4d923 to 46cee68 Compare March 28, 2024 10:28
@Just-Kiel Just-Kiel marked this pull request as ready for review April 17, 2024 09:45
@Just-Kiel Just-Kiel marked this pull request as draft April 17, 2024 09:45
@Just-Kiel Just-Kiel marked this pull request as ready for review April 17, 2024 09:46
@Just-Kiel Just-Kiel marked this pull request as draft April 17, 2024 09:47
@Just-Kiel Just-Kiel force-pushed the dev/nodeComputationDisplay branch from 46cee68 to 2057ffd Compare April 17, 2024 09:49
@Just-Kiel Just-Kiel force-pushed the dev/nodeComputationDisplay branch 5 times, most recently from ac04639 to a6e56cf Compare April 30, 2024 15:19
@fabiencastan fabiencastan dismissed stale reviews from cbentejac and themself June 1, 2024 09:39

changed

@fabiencastan fabiencastan marked this pull request as ready for review June 1, 2024 09:39
@fcastan fcastan force-pushed the dev/nodeComputationDisplay branch from a6e56cf to 1eb714f Compare June 1, 2024 09:43
@Just-Kiel Just-Kiel force-pushed the dev/nodeComputationDisplay branch from 1eb714f to 606e8e2 Compare June 11, 2024 09:18
@fabiencastan fabiencastan merged commit cbd2be0 into develop Jun 14, 2024
@fabiencastan fabiencastan deleted the dev/nodeComputationDisplay branch June 14, 2024 21:40
@fabiencastan fabiencastan added the feature new feature (proposed as PR or issue planned by dev) label Jun 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature new feature (proposed as PR or issue planned by dev)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants