Re: [Sugar-devel] Fix for AbiWord flicker and caret issues

2018-06-28 Thread Martin Sevior
Hi James,

Thank you! I'll try out your patch. From a quick review, I think your
mostly fixes the symptoms rather rather root cause.

Here is what I posted to the abiword-devel list.

I've also attached the complete patch to this email.

Cheers

Martin


--

Hi everyone,

I've finally fixed the problem of the flicker. It was caused by the final
call to gr_Caret.cpp::_blink( ) where we do
m_pG->flush();

This sends a signal to gtk3 to redraw the screen. In the process of
redrawing we call gr_Caret.cpp::setCoords() which in turn calls
gr_Caret.ccp::_erase() which in turn calls gr_Caret.cpp::_blink.cpp and we
get stuck in an infinite loop of redraws. Each redraw removes the caret
from the screen.

My solution was to
(a)
 -   m_pG->flush();
+if(bExplicit && !m_bCursorIsOn)
+m_pG->flush();

ie only call flush() if we really want to remove the caret from the screen.
If we simply remove the call to flush() there are circumstances where we
generate caret dirt. (After a focus event then moving the caret with the
arrow keys).
Then we also need in blink:
(b)
+struct timespec spec;

+clock_gettime(CLOCK_REALTIME, );
+
+UT_sint32 s  = spec.tv_sec;
+long ms = round(spec.tv_nsec / 1.0e6); // Convert nanoseconds to
milliseconds
+long this_time = 1000*s + ms;
+long time_between = this_time - m_iLastDrawTime;
+m_iLastDrawTime = this_time;
+if(time_between < 100)
+{
+m_iRetry++;
+if(!m_bCursorIsOn)
+{
+return;
+}
+}
+else
+{
+m_iRetry = 0;
+}

So if we have two rapid fire calls one to turn on the caret and the other
to remove, we short-circuit it and don't get our infinite loop.


On 28 June 2018 at 16:23, James Cameron  wrote:

> Thanks Martin.
>
> Your fix looks a little bit like mine; but I'm not quite sure what it
> is doing; could you comment on what your patch does?  Mine was on the
> AbiSource bug 13791, was taken by Debian and Ubuntu, and can be found
> with detailed explanation at
>
> https://sources.debian.org/patches/abiword/3.0.2-6/fix-flickering.patch/
>
> But it didn't work well with Wayland, according to Hubert.  I'm not
> targeting Wayland, so it was out of my scope.  When Wayland or a
> compositor are used, the redraws would often have no visible effect.
>
> I'd like to understand your fix as compared to mine.
>
> You might also ask for feedback from Abiword users on 13791.
>
> https://bugzilla.abisource.com/show_bug.cgi?id=13791
>
> The quickest way to reproduce Write flickering at the moment is to
> download Sugar on a Stick, for Fedora 28, which has the bug.
>
> On Thu, Jun 28, 2018 at 01:01:46PM +1000, Martin Sevior wrote:
> > Hi everyone,
> > I recently committed a fix to abiword head that I believe fixes a number
> of
> > abiword flickering and caret issues.
> >
> > I believe that this should also improve the performance of the
> application. It
> > was previously burning a lot of CPU in endless redraws.
> >
> > I haven't checked on Sugar Write which is built from abiwidget. I also
> haven't
> > checked that the multiple carets work either. I'd very much appreciate
> someone
> > from the sugar community trying it and providing feedback. Unfortunately
> it
> > appears that the abiword-devel mailing list has been put into
> hibernation but
> > if you reply to me via this mailing list I'll get your feedback and will
> work
> > on fixing problems you report.
> >
> > Sorry it has taken me so long to get round to this.
> >
> > Cheers
> >
> > Martin Sevior
> >
>
> > ___
> > Sugar-devel mailing list
> > Sugar-devel@lists.sugarlabs.org
> > http://lists.sugarlabs.org/listinfo/sugar-devel
>
>
> --
> James Cameron
> http://quozl.netrek.org/
>
Index: src/af/gr/gtk/gr_UnixCairoGraphics.cpp
===
--- src/af/gr/gtk/gr_UnixCairoGraphics.cpp	(revision 35465)
+++ src/af/gr/gtk/gr_UnixCairoGraphics.cpp	(working copy)
@@ -380,6 +380,7 @@
 {
 	UT_sint32 oldDY = tdu(getPrevYOffset());
 	UT_sint32 oldDX = tdu(getPrevXOffset());
+	
 	UT_sint32 newY = getPrevYOffset() + dy;
 	UT_sint32 newX = getPrevXOffset() + dx;
 	UT_sint32 ddx = -(tdu(newX) - oldDX);
Index: src/af/gr/xp/gr_Caret.cpp
===
--- src/af/gr/xp/gr_Caret.cpp	(revision 35465)
+++ src/af/gr/xp/gr_Caret.cpp	(working copy)
@@ -31,6 +31,11 @@
 #include "gr_Graphics.h"
 #include "gr_Painter.h"
 #include "ut_debugmsg.h"
+#include 
+#include 
+#include 
+#include 
+
 static const UT_uint32 CURSOR_DELAY_TIME = 10; // milliseconds
 
 #ifdef TOOLKIT_GTK_ALL
@@ -57,6 +62,11 @@
 		m_yPoint2(0),
 		m_pClr(NULL),
 		m_pG(pG),
+		m_iWindowWidth(0),
+		m_iWindowHeight(0),
+		m_worker(NULL),
+		m_enabler(NULL),
+		m_blinkTimeout(NULL),
 		m_nDisableCount(1),
 		m_bCursorBlink(true),
 		m_bCursorIsOn(false),
@@ -71,7 

Re: [Sugar-devel] Fix for AbiWord flicker and caret issues

2018-06-28 Thread James Cameron
Thanks Martin.

Your fix looks a little bit like mine; but I'm not quite sure what it
is doing; could you comment on what your patch does?  Mine was on the
AbiSource bug 13791, was taken by Debian and Ubuntu, and can be found
with detailed explanation at

https://sources.debian.org/patches/abiword/3.0.2-6/fix-flickering.patch/

But it didn't work well with Wayland, according to Hubert.  I'm not
targeting Wayland, so it was out of my scope.  When Wayland or a
compositor are used, the redraws would often have no visible effect.

I'd like to understand your fix as compared to mine.

You might also ask for feedback from Abiword users on 13791.

https://bugzilla.abisource.com/show_bug.cgi?id=13791

The quickest way to reproduce Write flickering at the moment is to
download Sugar on a Stick, for Fedora 28, which has the bug.

On Thu, Jun 28, 2018 at 01:01:46PM +1000, Martin Sevior wrote:
> Hi everyone,
> I recently committed a fix to abiword head that I believe fixes a number of
> abiword flickering and caret issues.
> 
> I believe that this should also improve the performance of the application. It
> was previously burning a lot of CPU in endless redraws.
> 
> I haven't checked on Sugar Write which is built from abiwidget. I also haven't
> checked that the multiple carets work either. I'd very much appreciate someone
> from the sugar community trying it and providing feedback. Unfortunately it
> appears that the abiword-devel mailing list has been put into hibernation but
> if you reply to me via this mailing list I'll get your feedback and will work
> on fixing problems you report.
> 
> Sorry it has taken me so long to get round to this.
> 
> Cheers
> 
> Martin Sevior
> 

> ___
> Sugar-devel mailing list
> Sugar-devel@lists.sugarlabs.org
> http://lists.sugarlabs.org/listinfo/sugar-devel


-- 
James Cameron
http://quozl.netrek.org/
___
Sugar-devel mailing list
Sugar-devel@lists.sugarlabs.org
http://lists.sugarlabs.org/listinfo/sugar-devel