Re: page that redraws endlessly

On Mon, Sep 05, 2011 at 12:41:55PM -0300, Jorge Arellano Cid wrote:
> On Mon, Sep 05, 2011 at 12:11:24AM +0200, Johannes Hofmann wrote:
> > On Sun, Sep 04, 2011 at 10:51:10PM +0200, Johannes Hofmann wrote:
> > > On Sun, Sep 04, 2011 at 10:05:22PM +0200, Johannes Hofmann wrote:
> > > > On Sun, Sep 04, 2011 at 07:15:09PM +0000, corvid wrote:
> > > > > Johannes wrote:
> > > > > > Thanks for the cleaned up test case. It's still remarkably complex.
> > > > > > I see the issue also with dillo-2.1.1. Nevertheless we should 
> > > > > > definately try to fix it. I will play with it a bit...
> > > > > 
> > > > > I wonder whether it is just the limit_text_width button problem dressed
> > > > > up in more complex clothing.
> > > > 
> > > > As funny as it sounds, hg bisect says it's rev 6a892184d498 where it
> > > > starts the redraw loop!
> > > > And indeed, if I do
> > > > 
> > > > --- a/src/prefsparser.cc  Sat Sep 03 01:15:33 2011 +0000
> > > > +++ b/src/prefsparser.cc  Sun Sep 04 22:04:18 2011 +0200
> > > > @@ -219,10 +219,11 @@
> > > >     setlocale(LC_NUMERIC, oldLocale);
> > > >     dFree(oldLocale);
> > > >  
> > > > -
> > > > +#if 0
> > > >     if (prefs.limit_text_width) {
> > > >        /* BUG: causes 100% CPU usage with <button> or <input type="image"> */
> > > >        MSG_WARN("Disabling limit_text_width preference (currently broken).\n");
> > > >        prefs.limit_text_width = FALSE;
> > > >     }
> > > > +#endif
> > > >  }
> > > > 
> > > > it loops with limit_text_width=NO and doesn't with limit_text_width=YES.
> > > 
> > > Ah wait! With limit_text_width=YES it's just not flickering. CPU is
> > > still at 100%.
> > 
> > 
> > The following test patch seems to stop it:
> > 
> > --- a/dw/ui.cc        Sat Sep 03 01:15:33 2011 +0000
> > +++ b/dw/ui.cc        Mon Sep 05 00:09:33 2011 +0200
> > @@ -268,7 +268,7 @@
> >     /**
> >      * \todo The argument to queueResize is not always true. (But this works.)
> >      */
> > -   resource->queueResize (true);
> > +   resource->queueResize (false);
> >  }
> > 
> > Maybe the comment isn't always right?
> 
>   Good shot!
> 
>   It also serves for another testcase I found.
> 
>   I don't know this part of the code well enough to say whether it
> is safe to commit before the release or not.

Actually I would suggest to commit attached patch.
I have to admit that I'm not 100% sure about the ui.cc change, but
it seems better than just always passing true or false.
The form.cc part makes sense, as limitTextWidth in
textblock.cc seems to matter only if layout->getUsesViewport ()
returns true, which is not the case for FltkFlatView in the
FltkComplexButton case.

corvid, Jorge, what do you think?

> 
>   FWIW, WRT complexity, if you resize the window with the patch,
> over the testcase, it is quite clear that the last phrase is not
> measured right.

Yes, there is an issue, but I don't think it's related to the
ComplexButton stuff.

<table>
  <tr>
    <td>
        <hr/>
      <div>
        words words words words words words words words words words words
        words words words words words words words words words words words
        words words words words words words words words words words words
        words words words words words words words words words words words
        <span style="white-space:nowrap">And words in a span</span>
      </div>
    </td>
</table>

looks similarily weird here, or am I missing something?

Cheers,
Johannes

_______________________________________________
Dillo-dev mailing list
[email protected]
http://lists.auriga.wearlab.de/cgi-bin/mailman/listinfo/dillo-dev

diff -r 108f6af13e0d dw/ui.cc
--- a/dw/ui.cc	Sat Sep 03 01:15:33 2011 +0000
+++ b/dw/ui.cc	Mon Sep 05 21:28:53 2011 +0200
@@ -266,9 +266,9 @@
                                                                int descent)
 {
    /**
-    * \todo The argument to queueResize is not always true. (But this works.)
+    * \todo Verify that this is correct.
     */
-   resource->queueResize (true);
+   resource->queueResize (resource->childWidget->extremesChanged ());
 }
 
 ComplexButtonResource::ComplexButtonResource ()
diff -r 108f6af13e0d src/form.cc
--- a/src/form.cc	Sat Sep 03 01:15:33 2011 +0000
+++ b/src/form.cc	Mon Sep 05 21:28:53 2011 +0200
@@ -851,7 +851,7 @@
       Embed *embed;
       char *name, *value;
 
-      page = new Textblock (prefs.limit_text_width);
+      page = new Textblock (false);
       page->setStyle (html->styleEngine->backgroundStyle ());
 
       ResourceFactory *factory = HT2LT(html)->getResourceFactory();
diff -r 108f6af13e0d src/prefsparser.cc
--- a/src/prefsparser.cc	Sat Sep 03 01:15:33 2011 +0000
+++ b/src/prefsparser.cc	Mon Sep 05 21:28:53 2011 +0200
@@ -218,11 +218,4 @@
    // restore the old numeric locale
    setlocale(LC_NUMERIC, oldLocale);
    dFree(oldLocale);
-
-
-   if (prefs.limit_text_width) {
-      /* BUG: causes 100% CPU usage with <button> or <input type="image"> */
-      MSG_WARN("Disabling limit_text_width preference (currently broken).\n");
-      prefs.limit_text_width = FALSE;
-   }
 }