D20029: Capture counting corrected

[Wolfgang Reissenberger]

wreissenberger requested review of this revision.
wreissenberger added a comment.


  Please check my comments, from my point of view everything should be fine.

INLINE COMMENTS

> TallFurryMan wrote in scheduler.cpp:1455
> A null pointer exception here? I see no direct reason for it (in setCurrentJob), nor for the removal of the current job if no job is left to run, could you elaborate on the consequences?

The problem occurs in line 4675 (sic!):
`if (currentJob->getRepeatsRemaining() == 0)`
when trying to find the next job. `evaluateJobs()` is called before and sets `currentJob` to `nullptr` if no jobs are left over to be executed.

> TallFurryMan wrote in scheduler.cpp:5212
> I don't understand why you removed that line. Variable "captures_completed" can't always equal zero when not remembering progress: runtime events may abort the job, but still number of captures must be retained. This would cause areJobCapturesComplete to flip to true immediately...

This is irrelevant, since the number of completed frames is only changed in the case that we remember the job progress (see lines 5266ff). In theory we could leave it, but it is not used in the case that we do not remember the job progress.

This together with lines 5266ff resolve the second bullet point of 1. In the old implementation, the number of completed frames has been added for each of the sequence job entries and counted the same value multiply.

> TallFurryMan wrote in scheduler.cpp:5268
> If the algorithm is OK, I would expect totalCompletedCount == captures_completed if rememberJobProgress is false.
> I see the "optimization" andI agree on the principle, but I'd prefer a debug assertion here in a else. This change is introducing a dependency between the end and beginning of the algorithm, while we strive to clarify it.

An assertion does not make sense for the else branch, since in this case the number of completed frames is already set in the SequenceJob.

REPOSITORY
  R321 KStars

REVISION DETAIL
  https://phabricator.kde.org/D20029

To: wreissenberger, mutlaqja, TallFurryMan
Cc: kde-edu, narvaez, apol
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-edu/attachments/20190325/177cb9ff/attachment.html>