On 2009-03-17 23:20, Genete closed the bug 2652974 “Canvas is not refreshed” as fixed since the problem disappeared by reverting r2326. I was wondering whether it is correct to considered this problem fixed. Reverting r2326 is certainly a work around, but, to me, the code is either wrong and thus should be removed completely (not remain commented out), or the problem is actually somewhere else, and reverting r2326 only hides the problem.
Note that with this “fix” we have lost (or not regained) the onion skins of +1 and -1 seconds. As such, this fix has a cost. Or, is no one interested in these onionskins?
I guess I’m proposing to change the state of this bug from fixed to postponed. Postponed until someone can take a good look at it…
For instance, I suspect the (independent) moving/additions of
if(!onionskin)
return; in set_onion_skin (synfig-studio/src/gtkmm/workarea.cpp, line 126) by Genete in r2354, has already helped deal with this problem.
And, I am, in the context of this bug, also suspisous of the
refreshes+=5;
throughout the workarea.cpp file. I suspsect that “5 refreshes” == “one frame for previous keyframe + one frame for -1 second + one frame for now + one frame for +1 second + one frame for next keyframe”.
So, (after my ramblings), what are you guys thinking…?
I reverted to the previous state of that commit because the program was turned totally useless. I know (and I knew( that it was a WIP from pabs of a missing feature but as he was absent those days I decided to leave the “code as it was” (as least working).
Also I tried to just move/add the:
if(!onionskin)
return;
instead of comment those lines but it didn’t work either (I don’t know why).
It seems that the +/-1 second onionskin was commented out since the first release of the file. Honestly I don’t know what else should be added to enable a fixed +/-1 second onionskin. Maybe pabs or you know what’s needed.
Regarding to the onionskin I really would prefer to be able to manually set them in a fixed position (like a keyframe) and/or in a relative position from the cursor (but customizable, not always +/-1 second).
So I see it more like a non finished feature rather than a bug.
-G
Please don’t get me wrong. I fully support your action of reverting the change. We needed that workaround to keep synfig running.
My point is that marking this bug as fixed, suggests a finality to this problem that isn’t there. The code is still there, and it still seems reasonable to uncomment it, and we still don’t know why that will get us in trouble. As far as I know we still don’t even have simple .sif file with which to directly trigger this bug. It usually takes me a while to fiddle with a .sif file to get it to freeze up the rendering.
We know at least the following:
It has to do with the tile renderer.
It has to with onionskin queue.
It occurs, however, when onionskin off.
If we want to use a (configurable) unionskin feature in the future, we will need to understand what’s going on why.
Sure!. But it is more probably that I mess it up than I fix it
The commit from pabs that uncommented it said: “WIP” so it means that something else has to be done to make it working. Or not… because I suspect that the problem still there and is related to collisions with the threads of the tile render.
Hmmm… strange. I’ve uncommented the lines 129 and 130 of workarea.cpp of the current release and it seems to not hang up anymore. I usually tested the bug with the eyes.sifz from the examples and now I cannot make it freeze. And effectively it produces 5 onionskins. Only at tile render, but the scanline should work as well.
Having more onion skin would be very helpful. Specially if you can control the position and the transparency were not uniform (more transparent if it is far away).
Maybe I fixed it up with the r2354? I didn’t do an extensive test after revert pabs changes, in fact.
-G
