Implement wxAnimation GetFrameCount() GetDelay() GetFrame() for native GTK (PR #27009)
RobertRoeb
Added test to sample
Updated docs to remove limitation
You can view, comment on, or merge this pull request online at:
https://github.com/wxWidgets/wxWidgets/pull/27009
Commit Summary
- 72543ce Implement wxAnimation GetFrameCount() GetDelay() and GetFrame() for native GTK
File Changes
(4 files)
- M include/wx/gtk/private/animate.h (10)
- M interface/wx/animate.h (6)
- M samples/animate/anitest.cpp (9)
- M src/gtk/animate.cpp (154)
Patch Links:
- https://github.com/wxWidgets/wxWidgets/pull/27009.patch
- https://github.com/wxWidgets/wxWidgets/pull/27009.diff
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
Maarten
@MaartenBent commented on this pull request.
In samples/animate/anitest.cpp:
> + wxSizer *frameSizer = new wxBoxSizer( wxHORIZONTAL );
+ sz->Add( frameSizer, 0, wxALL|wxCENTER, 10 );
+ for (unsigned int i = 0; i < throbber.GetFrameCount(); i++)
+ {
+ wxImage image = throbber.GetFrame( i );
+ frameSizer->Add( new wxStaticBitmap(this, -1, wxBitmap( image ) ) );
+ }
Hi. This sample can also be used to open other animations, and then it still shows the throbber frames.
For bigger images with many frames, showing all frames next to each other will be problematic. It won't fit. So I don't think this addition is necessary.
You can already use the image sample to show any image..., and for gif and other animations it will show the number of frames in the statusbar. And you can use the menu or ctrl+< > to go step through the frames.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
VZ
@vadz commented on this pull request.
Thanks, this should be useful, but I agree with the comment about showing all frames in the sample, it would be better to just show the number of frames in the status bar. If you really want to be able to test GetFrame(), add a spin control allowing to select the index of the frame to show just one at a time, but I'm fine with just removing it. OTOH adding even a very simple unit test for this would be really useful.
And if we could reuse some existing code to copy from GdkPixBuf, it would be great.
Also, it's a pity to repeat the not quite trivial loop over frames 3 times, but I guess abstracting it would be difficult?
> @@ -166,9 +166,159 @@ bool wxAnimationGTKImpl::Load(wxInputStream &stream, wxAnimationType type)
return data_written;
}
-wxImage wxAnimationGTKImpl::GetFrame(unsigned int WXUNUSED(frame)) const
+unsigned int wxAnimationGTKImpl::GetFrameCount() const
+{
+ GTimeVal start_time;
+ g_get_current_time(&start_time);
+ GdkPixbufAnimationIter *iter = gdk_pixbuf_animation_get_iter(m_pixbuf, &start_time);
As usual:
⬇️ Suggested change- GdkPixbufAnimationIter *iter = gdk_pixbuf_animation_get_iter(m_pixbuf, &start_time); + wxGtkObject<GdkPixbufAnimationIter> iter = gdk_pixbuf_animation_get_iter(m_pixbuf, &start_time);
and remove g_object_unref() at the end.
> -wxImage wxAnimationGTKImpl::GetFrame(unsigned int WXUNUSED(frame)) const
+unsigned int wxAnimationGTKImpl::GetFrameCount() const
+{
+ GTimeVal start_time;
+ g_get_current_time(&start_time);
+ GdkPixbufAnimationIter *iter = gdk_pixbuf_animation_get_iter(m_pixbuf, &start_time);
+
+ int frame_count = 0;
+ int total_delay_ms = 0;
+
+ while (true)
+ {
+ frame_count++;
+
+ int delay = gdk_pixbuf_animation_iter_get_delay_time(iter);
+ if (delay <= 0) break; // static state or an error
Minor, but please don't put compound statement bodies on the same line:
⬇️ Suggested change- if (delay <= 0) break; // static state or an error + if (delay <= 0) + break; // static state or an error
> + delay = gdk_pixbuf_animation_iter_get_delay_time(iter); + if (delay <= 0) break; // static state or an error + + total_delay_ms += delay; + + GTimeVal next_time = start_time; + g_time_val_add(&next_time, total_delay_ms * 1000); // microseconds + gdk_pixbuf_animation_iter_advance(iter, &next_time); + } + + g_object_unref(iter); + + return delay; +} + +static void CopyImageData(
Don't we already have the code for converting from GdkPixbuf to wxImage somewhere? I think we should...
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
RobertRoeb
@RobertRoeb pushed 1 commit.
- 85b92d0 Use GtkObject<>, factor out code, remove whitespace
—
View it on GitHub or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
RobertRoeb
@RobertRoeb pushed 1 commit.
- 4300d15 Move decl into gtk/private.h and reuse code from gtk/bitmap.cpp
—
View it on GitHub or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
RobertRoeb
@RobertRoeb pushed 1 commit.
- 270c60c Remove test for wxAnimation::GetFrame()
—
View it on GitHub or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
RobertRoeb
@RobertRoeb pushed 1 commit.
- fd495a4 Removed whitespace and added DLLEXPORT statement
—
View it on GitHub or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
paulcor
@paulcor commented on this pull request.
> + int total_delay_ms = 0;
+
+ for (unsigned int i = 0; i < frame; i++)
+ {
+ delay = gdk_pixbuf_animation_iter_get_delay_time(iter);
+ if (delay <= 0)
+ break; // static state or an error
+
+ total_delay_ms += delay;
+
+ GTimeVal next_time = start_time;
+ g_time_val_add(&next_time, total_delay_ms * 1000); // microseconds
+ gdk_pixbuf_animation_iter_advance(iter, &next_time);
+ }
+
+ GdkPixbuf *buf = gdk_pixbuf_animation_iter_get_pixbuf(iter);
You could have saved yourself a whole lot of trouble and just done this:
g_object_ref(buf);
return wxBitmap(buf).ConvertToImage();
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
RobertRoeb
@RobertRoeb commented on this pull request.
> + int total_delay_ms = 0;
+
+ for (unsigned int i = 0; i < frame; i++)
+ {
+ delay = gdk_pixbuf_animation_iter_get_delay_time(iter);
+ if (delay <= 0)
+ break; // static state or an error
+
+ total_delay_ms += delay;
+
+ GTimeVal next_time = start_time;
+ g_time_val_add(&next_time, total_delay_ms * 1000); // microseconds
+ gdk_pixbuf_animation_iter_advance(iter, &next_time);
+ }
+
+ GdkPixbuf *buf = gdk_pixbuf_animation_iter_get_pixbuf(iter);
Damn
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
VZ
@vadz commented on this pull request.
> + int total_delay_ms = 0;
+
+ for (unsigned int i = 0; i < frame; i++)
+ {
+ delay = gdk_pixbuf_animation_iter_get_delay_time(iter);
+ if (delay <= 0)
+ break; // static state or an error
+
+ total_delay_ms += delay;
+
+ GTimeVal next_time = start_time;
+ g_time_val_add(&next_time, total_delay_ms * 1000); // microseconds
+ gdk_pixbuf_animation_iter_advance(iter, &next_time);
+ }
+
+ GdkPixbuf *buf = gdk_pixbuf_animation_iter_get_pixbuf(iter);
@RobertRoeb Will you update this to use wxBitmap?
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
RobertRoeb
@RobertRoeb pushed 1 commit.
- 104b3b8 Use wxBitmap() constructor
—
View it on GitHub or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
paulcor
@paulcor commented on this pull request.
In include/wx/gtk/private/animate.h:
> @@ -12,6 +12,7 @@ #define _WX_GTK_PRIVATE_ANIMATEH__ #include "wx/private/animate.h" +#include <glib.h>
This doesn't appear to be needed.
In samples/animate/anitest.cpp:
> @@ -186,6 +186,7 @@ MyFrame::MyFrame(wxWindow *parent,
}
sz->Add(m_animationCtrl, wxSizerFlags().Centre().Border());
+
Unnecessary white space change.
> +
+ for (unsigned int i = 0; i < frame; i++)
+ {
+ delay = gdk_pixbuf_animation_iter_get_delay_time(iter);
+ if (delay <= 0)
+ break; // static state or an error
+
+ total_delay_ms += delay;
+
+ GTimeVal next_time = start_time;
+ g_time_val_add(&next_time, total_delay_ms * 1000); // microseconds
+ gdk_pixbuf_animation_iter_advance(iter, &next_time);
+ }
+
+ GdkPixbuf *buf = gdk_pixbuf_animation_iter_get_pixbuf(iter);
+ return wxBitmap( buf ).ConvertToImage();
You have to add a reference to the pixbuf, because the wxBitmap dtor is going to un-ref it.
> @@ -70,6 +70,51 @@ static void MaskToAlpha(GdkPixmap* mask, GdkPixbuf* pixbuf, int w, int h) } #endif +static void CopyPixbufDataToImage(
All of these changes to src/gtk/bitmap.cpp are unnecessary.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
RobertRoeb
@RobertRoeb pushed 1 commit.
- 7fc9c83 Added g_object_ref and removed unnecessary changes
—
View it on GitHub or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
VZ
vadz left a comment (wxWidgets/wxWidgets#27009)Thanks for the updates, I had already done the same thing and a bit more locally, will push it soon.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
VZ
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.