Bug 151456 - AnimatedImages in Basic Dialog : StepTime is ignored
Summary: AnimatedImages in Basic Dialog : StepTime is ignored
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: BASIC (show other bugs)
Version:
(earliest affected)
Inherited From OOo
Hardware: All All
: medium minor
Assignee: Julien Nabet
URL:
Whiteboard: target:24.8.0 target:24.2.0.0.beta2
Keywords:
Depends on:
Blocks: Images-Animated
  Show dependency treegraph
 
Reported: 2022-10-10 15:58 UTC by Jurassic Pork
Modified: 2023-12-15 08:21 UTC (History)
2 users (show)

See Also:
Crash report or crash signature:


Attachments
Zip containing a Writer document and 6 images (91.43 KB, application/x-zip-compressed)
2022-10-10 16:01 UTC, Jurassic Pork
Details
bt with debug symbols (14.70 KB, text/plain)
2022-10-11 20:24 UTC, Julien Nabet
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Jurassic Pork 2022-10-10 15:58:37 UTC
Description:
Zip containing a Writer document and 6 images
Setting (c.s.s.awt.XAnimatedImages) StepTime does not change the rate of images display, at least on Windows 10.
Attachment is a zip containing AnimatedProblems.odt and 6 png images. Unzip all in the same folder. Click the button to run the macro BASIC and open the dialog with animation.
StepTime should be 2000ms between two images.

Steps to Reproduce:
1.Setting (c.s.s.awt.XAnimatedImages) StepTime does not change the rate of images display.
2.Attachment is a zip containing AnimatedProblems.odt and 6 png images. Unzip all in the same folder. 
Run the macro : StepTime should be 2000ms between two images.


Actual Results:
StepTime seems to be 100ms

Expected Results:
StepTime should be 2000ms 


Reproducible: Always


User Profile Reset: No



Additional Info:
It turns out that the control peer actually calls the respective VCL class (Throbber) to change the step time:

AnimatedImagesPeer::setProperty()

case BASEPROPERTY_STEP_TIME:
{
    sal_Int32 nStepTime( 0 );
    if ( i_value >>= nStepTime )
         pThrobber->setStepTime( nStepTime );
    break;
}

Unfortunately, Throbber::setStepTime() simply stores that value in a member variable 

void  setStepTime( sal_Int32 nStepTime )  { mnStepTime = nStepTime; }

that member variable is never used to actually set the AutoTimer timeout: the timeout is only set on the Throbber constructor, where the timeout value is set to a default value of 100:

Throbber::Throbber( Window* i_parentWindow, WinBits i_style, const ImageSet i_imageSet )
    :ImageControl( i_parentWindow, i_style )
    ,mbRepeat( sal_True )
    ,mnStepTime( 100 )
    ,mnCurStep( 0 )
    ,mnStepCount( 0 )
    ,meImageSet( i_imageSet )
{
    maWaitTimer.SetTimeout( mnStepTime );
    maWaitTimer.SetTimeoutHdl( LINK( this, Throbber, TimeOutHdl ) );

    SetScaleMode( ImageScaleMode::None );
    initImages();
}

Possible solutions:

a) make Throbber::setStepTime() update the AutoTimer's timeout, but what if the Throbber is running (== the timer is active)?
b) reset the AutoTimer's timeout in Throbber::start(), that is, when the animation is started; this will have the effect that setting the step time while the animation is running will only take effect after it restarts
c) ....

solution to be tested : 
in the file vcl/source/control/throbber.cxx  change the start method like this: 
void Throbber::start()
{
    maWaitTimer.SetTimeout( mnStepTime );
    maWaitTimer.Start();
}
Comment 1 Jurassic Pork 2022-10-10 16:01:43 UTC
Created attachment 182951 [details]
Zip containing a Writer document and 6 images
Comment 2 Julien Nabet 2022-10-11 20:24:18 UTC
Created attachment 182979 [details]
bt with debug symbols

Just for the record, here's a bt from the constructor of Throbber.
Indeed, I don't know how the StepTime may be taken into account.
Comment 3 Buovjaga 2023-02-21 09:32:53 UTC
Repro with file, thanks for the code pointers.

Arch Linux 64-bit, X11
Version: 7.6.0.0.alpha0+ (X86_64) / LibreOffice Community
Build ID: 0bcce236059ae68c6dcc7bce8ceaec5d39c28f1c
CPU threads: 8; OS: Linux 6.1; UI render: default; VCL: kf5 (cairo+xcb)
Locale: fi-FI (fi_FI.UTF-8); UI: en-US
Calc: threaded
Built on 21 February 2023
Comment 4 Julien Nabet 2023-12-14 12:40:51 UTC
https://gerrit.libreoffice.org/c/core/+/160771

There were 2 problems:
1) As the author suggested, we had to add:
maWaitTimer.SetTimeout( mnStepTime );
in void Throbber::start()
Thank you Jurassik Pork! :-)

2) There was a regression on the test of "pThrobber" since 51e97b2ffd8f0ae0591d1880d621cba4596583b7
toolkit: first cut at switching to VclPtr.
Comment 5 Commit Notification 2023-12-14 21:59:04 UTC
Julien Nabet committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/fdc87dd56548622e13353b4cf9864232ee0110fb

tdf#151456: AnimatedImages in Basic Dialog : StepTime is ignored

It will be available in 24.8.0.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 6 Julien Nabet 2023-12-14 22:00:16 UTC
Fix for 24.2 is on review here:
https://gerrit.libreoffice.org/c/core/+/160798
Comment 7 Commit Notification 2023-12-15 08:21:07 UTC
Julien Nabet committed a patch related to this issue.
It has been pushed to "libreoffice-24-2":

https://git.libreoffice.org/core/commit/2324cafba1d41374885cbc2e52b7449de3d924bf

tdf#151456: AnimatedImages in Basic Dialog : StepTime is ignored

It will be available in 24.2.0.0.beta2.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.