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, &spec);
+
+ 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 <[email protected]> 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
> > [email protected]
> > 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 <inttypes.h>
+#include <math.h>
+#include <stdio.h>
+#include <time.h>
+
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 +81,9 @@
m_bRemote(false),
m_clrRemote(0,0,0),
m_sID(""),
- m_iCaretNumber(0)
+ m_iCaretNumber(0),
+ m_iLastDrawTime(0),
+ m_iRetry(0)
{
UT_WorkerFactory::ConstructMode outMode = UT_WorkerFactory::NONE;
m_worker = static_cast<UT_Timer *>(UT_WorkerFactory::static_constructor
@@ -98,6 +110,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),
@@ -112,7 +129,9 @@
m_bRemote(true),
m_clrRemote(0,0,0),
m_sID(sId),
- m_iCaretNumber(0)
+ m_iCaretNumber(0),
+ m_iLastDrawTime(0),
+ m_iRetry(0)
{
UT_WorkerFactory::ConstructMode outMode = UT_WorkerFactory::NONE;
m_worker = static_cast<UT_Timer *>(UT_WorkerFactory::static_constructor
@@ -146,9 +165,11 @@
void GR_Caret::s_work(UT_Worker * _w)
{
GR_Caret * c = static_cast<GR_Caret *>(_w->getInstanceData());
-
+ UT_DEBUGMSG((" Caret timer called Disable Count = %d \n",c->m_nDisableCount));
if (c->m_nDisableCount == 0)
+ {
c->_blink(false);
+ }
}
/** One-time enabler. */
@@ -171,9 +192,12 @@
void GR_Caret::s_blink_timeout(UT_Worker * _w)
{
+ return;
+ /*
GR_Caret * c = static_cast<GR_Caret *>(_w->getInstanceData());
if (c->isEnabled())
c->disable();
+ */
}
UT_uint32 GR_Caret::_getCursorBlinkTime() const
@@ -271,13 +295,17 @@
// If the caret is already enabled, just return
if (m_nDisableCount == 0)
+ {
+ xxx_UT_DEBUGMSG(("Don't emable disable Count is already zero \n"));
return;
-
+ }
// Check to see if we still have pending disables.
--m_nDisableCount;
- if (m_nDisableCount)
+ if (m_nDisableCount != 0)
+ {
+ xxx_UT_DEBUGMSG(("Don't emable, disable Count has not reached zero \n"));
return;
-
+ }
// stop pending enables; in 10 ms, really enable blinking.
m_enabler->stop();
m_enabler->start();
@@ -327,11 +355,12 @@
{
if(m_bRecursiveDraw)
{
- xxx_UT_DEBUGMSG(("Doing recursive Erase! - abort \n"));
+ xxx_UT_DEBUGMSG(("Doing recursive Just Erase! - abort \n"));
return;
}
if (m_bCursorIsOn && (((xPoint -m_pG->tlu(2)-1) <= m_xPoint) && (xPoint >= (m_xPoint-m_pG->tlu(2))-1)) && ((yPoint - m_pG->tlu(1)) <= m_yPoint) && (yPoint >= (m_yPoint - m_pG->tlu(1))))
{
+ xxx_UT_DEBUGMSG(("Doing Just Erase! now \n"));
m_pG->restoreRectangle(m_iCaretNumber*3+0);
if(m_bSplitCaret)
{
@@ -339,7 +368,7 @@
m_pG->restoreRectangle(m_iCaretNumber*3+2);
m_bSplitCaret = false;
}
- m_bCursorIsOn = !m_bCursorIsOn;
+ m_bCursorIsOn = false;
m_nDisableCount = 1;
}
}
@@ -359,7 +388,27 @@
}
if (!m_bPositionSet)
return;
+ struct timespec spec;
+ clock_gettime(CLOCK_REALTIME, &spec);
+
+ 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;
+ }
m_bRecursiveDraw = true;
GR_Painter painter (m_pG, false);
m_bRecursiveDraw = false;
@@ -383,6 +432,7 @@
if (m_bCursorIsOn)
{
+ xxx_UT_DEBUGMSG(("Clear Caret reTry %d \n",m_iRetry));
m_pG->restoreRectangle(m_iCaretNumber*3+0);
if(m_bSplitCaret)
@@ -391,6 +441,7 @@
m_pG->restoreRectangle(m_iCaretNumber*3+2);
m_bSplitCaret = false;
}
+ m_bCursorIsOn = false;
}
else
{
@@ -416,7 +467,9 @@
m_yPoint+m_pG->tlu(1),
m_pG->tlu(5),
m_iPointHeight+m_pG->tlu(2));
+ m_bRecursiveDraw = false;
m_pG->allCarets()->JustErase(m_xPoint,m_yPoint);
+ m_bRecursiveDraw = true;
m_pG->saveRectangle(r0,m_iCaretNumber*3+0);
if((m_xPoint != m_xPoint2) || (m_yPoint != m_yPoint2))
@@ -449,7 +502,7 @@
if(m_bCaret1OnScreen)
{
// draw the primary caret
- xxx_UT_DEBUGMSG(("blink cursor turned on \n"));
+ xxx_UT_DEBUGMSG(("Draw Caret reTry %d \n",m_iRetry));
UT_sint32 x1 = m_xPoint + iDelta * m_pG->tlu(1);
UT_sint32 x2 = m_xPoint;
@@ -466,6 +519,7 @@
m_yPoint + m_pG->tlu(1),
x2,
m_yPoint + m_iPointHeight + m_pG->tlu(1));
+ m_bCursorIsOn = true;
}
if(m_bSplitCaret)
@@ -502,6 +556,7 @@
m_xPoint + m_pG->tlu(2),
m_yPoint + m_pG->tlu(2));
}
+ m_bCursorIsOn = true;
}
// Now we deal with the secondary caret needed on ltr-rtl boundary
@@ -560,6 +615,7 @@
m_xPoint2 /*- m_pG->tlu(1)*/,
m_yPoint2 + m_pG->tlu(2));
}
+ m_bCursorIsOn = true;
}
}
@@ -566,11 +622,11 @@
}
- m_bCursorIsOn = !m_bCursorIsOn;
m_pG->setColor(oldColor);
m_bRecursiveDraw = false;
}
- m_pG->flush();
+ if(bExplicit && !m_bCursorIsOn)
+ m_pG->flush();
}
/*!
Index: src/af/gr/xp/gr_Caret.h
===================================================================
--- src/af/gr/xp/gr_Caret.h (revision 35465)
+++ src/af/gr/xp/gr_Caret.h (working copy)
@@ -121,6 +121,8 @@
UT_RGBColor m_clrRemote;
std::string m_sID;
UT_sint32 m_iCaretNumber;
+ long m_iLastDrawTime;
+ UT_sint32 m_iRetry;
};
class ABI_EXPORT GR_CaretDisabler
Index: src/af/util/unix/ut_unixTimer.cpp
===================================================================
--- src/af/util/unix/ut_unixTimer.cpp (revision 35465)
+++ src/af/util/unix/ut_unixTimer.cpp (working copy)
@@ -68,7 +68,7 @@
xxx_UT_DEBUGMSG(("ut_unixTimer.cpp: timer fired\n"));
if (pTimer) {
pTimer->fire();
- return true;
+ return TRUE;
}
return 0;
}
_______________________________________________
Sugar-devel mailing list
[email protected]
http://lists.sugarlabs.org/listinfo/sugar-devel
