- Dillo-dev - lists.mailman3.com

forms related segfault

by jcid＠dillo.org

Hi, The big patch for html.cc getting rid of the linkblock and html structures, merging them in a single class with link-receiver inside, and also enabling freeing of the resources formerly stored in the linkblock, is commited! :-) The amazing thing is that it seems to work. ;) On Fri, Nov 09, 2007 at 09:34:33AM -0300, Jorge Arellano Cid wrote: > Hi, > > On Fri, Nov 09, 2007 at 09:09:26AM -0300, Jorge Arellano Cid wrote: > > [...] > > As a side effect of flickering, I noticed several times how some > > non existent inputs (text entries and buttons) were phantom-rendered > > during updates (i.e. appeared on screen for a few jiffies). > > > > [...] > > For instance, enable flickering (disable double-buffer and > internal buffer), then go to: > > 1. www.google.com //and then to > 2. chlamydia.fs.ei.tum.de/pub/DragonFly/packages/stable/DragonFly-1.10/All/ > > While loading the second URL you'll see the phantom-rendering > of the text input and the two buttons that google uses. Although now we have memory-freeing hooked, the phantom rendering still occurs... > > Now, I thought this was to be deleted from the call in web.cc > (as a children of the top widget or something), but there's > another way to do it that looks like the original idea. Actually, it doesn't look like a good idea. What seems proper is that Dw internally deletes its widget tree, and DilloHtml deletes its non-dw state (as now). > In html parsing we have a structure called the linkblock. This > contains all the data that is needed after the page is parsed and > rendered (mainly forms data), and also dw widgets! > > In html.hh you'll find DilloHtmlLB. Within DilloHtmlLB there's > a vector of DilloHtmlForm, which has a vector of DilloHtmlInput > which holds widgets. > > The linkblock is designed to be freed when the displayed dw is > deleted, but this hook is not currently operative! (this is the > warning you get at html.cc compile time about Html_lb_free not > being called). Well, now there's no linkblock, and everything is inside the DilloHtml class (much cleaner). Memory handling is hooked and it works. > Anyway, when the linkblock-free callback is hooked, those > widgets can be destroyed from Html_lb_free(). Which is mainly > implemented but not yet tested. My mistake: this part is for freeing non-dw data. What _seems_ to happen is that the "Embed" widgets are not being deleted (not yet confirmed though). Feel free to investigate! :-) -- Cheers Jorge.-

Nov. 10, 2007

[patch] fix bug in new scrolling code

by Johannes.Hofmann＠gmx.de

Hi, when scrolling using the down arrow key I saw occasionally screen corruptions. The patch below fixes this for me. BTW. the following little program helped me debugging this and might be useful for others with too fast machines. It hogs the CPU, so dillo redraws are much slower and easier to follow: int main(int argc, char **argv) { for (;;); } Cheers, Johannes diff --git a/dw/fltkviewport.cc b/dw/fltkviewport.cc --- a/dw/fltkviewport.cc +++ b/dw/fltkviewport.cc @@ -311,8 +311,11 @@ void FltkViewport::scroll (int dx, int d { int hdiff = vscrollbar->visible () ? SCROLLBAR_THICKNESS : 0; int vdiff = hscrollbar->visible () ? SCROLLBAR_THICKNESS : 0; - int oldScrollX = scrollX; - int oldScrollY = scrollY; + /* multiple calls to scroll can happen before a redraw occurs. + * scrollDX / scrollDY can therefore be non-zero here. + */ + int oldScrollX = scrollX - scrollDX; + int oldScrollY = scrollY - scrollDY; if (dx == 0 && dy == 0) { return;

Nov. 10, 2007

[patch] make partial redraw requests work

by jcid＠dillo.org

On Fri, Nov 09, 2007 at 10:51:11PM +0100, Johannes Hofmann wrote: > Argh, forget about my last patch please. It was much too complicated. > fltk does all that for us! > The following looks much simpler, doesn't it :-) Yes! > Sorry for the confusion, It looked quite nice! ;-) Reverted and applied in CVS. -- Cheers Jorge.-

Nov. 10, 2007

[patch] make partial redraw requests work

by jcid＠dillo.org

On Fri, Nov 09, 2007 at 08:22:41PM +0100, Johannes Hofmann wrote: > Hi, > > this patch adds set(), isEmpty(), and merge() methods to > core::Rectangle. > These methods are then used to maintain a rectangle that contains > all screen areas for which a redraw was requested. > When fltk really draws to the screen, the drawing is limited to this > area. > The most obvious improvement is visible on pages with very large > images that take some time to load. While the image is loaded the > rest of the page is no longer flickering - at least if there is no > other stuff that requests complete redraws. Committed! -- Cheers Jorge.-

Nov. 10, 2007

[patch] enable partial image drawing

by jcid＠dillo.org

On Fri, Nov 09, 2007 at 08:14:47PM +0100, Johannes Hofmann wrote: > Hi, > > here is a first patch to reduce redrawing in dillo-fltk. > It just enables code that already existed but was commented out and > allows drawing of parts of an image only. Yes, I wrote that code, and it worked most of the time. It had problems when scrolling an image that was still loading. Now with the new scrolling it showed no problems in my quick tests. > I added some additional checks. With these checks I did not see any > negative effects. Commited. If somebody can test scrolling a downloading image over a slow line (e.g. phone) it'd be welcomed. -- Cheers Jorge.-

Nov. 9, 2007

[patch] make partial redraw requests work

by Johannes.Hofmann＠gmx.de

Argh, forget about my last patch please. It was much too complicated. fltk does all that for us! The following looks much simpler, doesn't it :-) --- a/dw/fltkviewbase.cc Wed Nov 07 22:39:38 2007 +0100 +++ b/dw/fltkviewbase.cc Fri Nov 09 22:40:14 2007 +0100 @@ -223,9 +223,13 @@ void FltkViewBase::finishDrawing (core:: void FltkViewBase::queueDraw (core::Rectangle *area) { - /** \bug Inefficient */ - redraw (DAMAGE_EXPOSE); + redraw(::fltk::Rectangle( + translateCanvasXToViewX(area->x), + translateCanvasYToViewY(area->y), + area->width, + area->height)); + } Sorry for the confusion, Johannes

Nov. 9, 2007

Re: new browser windows slower than first one

by jcid＠dillo.org

Hi, On Fri, Nov 09, 2007 at 08:59:22PM +0100, Johannes Hofmann wrote: > Hi Jorge, > > On Wed, Nov 07, 2007 at 06:56:22PM -0300, Jorge Arellano Cid wrote: > > Hi Johannes, > > > > On Wed, Nov 07, 2007 at 09:46:25PM +0100, Johannes Hofmann wrote: > > *snip* > > > > > > The last point is more difficult to improve. For example think of a > > > long page containing just text. Every time a new chunk comes in over > > > the net, dillo creates a redraw request for the whole screen, > > > even though there is already enough text to fill the screen so > > > that the new data does not actually change the layout of the part > > > that is currently visible. > > > > > > But how can we know, whether new data changes the layout of the > > > currently visible part or not? > > > > Please excuse me if I'm naive on this subject (it's not my main > > area of expertise). > > > > I think that if the visible widgets don't change their size, > > and the just-added widgets are out of sight, we can delay the > > redraw (maybe setting a damage flag or whatever it is that works > > on the out-of-sight widgets), and not request a full redraw. > > I think we would not even need to mark them. They get redrawn > anyway when they come back in sight. Great. > I see one problem however. > Many pages just consist of one large textblock. And part of this > textblock is obviously visible while loading the page. > But with every chunk of data from the net the size of this textblock > will change and we would redraw it, even though the part of it that > is visible did not change. > I think we need a solution for this common case. Yes, it's true. > I already thought > about checking whether something changed at a word by word level, > but this feels a bit like overkill. But it could work. Let's say we keep the convas coordinate of the last line in a textblock in a state variable of the object instance. If that line is out of sight we can leave: mustQueueResize = false and be more careful about calling Textblock::flush(). > > If that approach is too complex to implement, then maybe it's > > not worth the effort. > > > > What do you think? > > Maybe. The actual redraws are done when fltk is idle otherwise. > So many redraw request are collapsed into one already. > > However redraws are much more expensive with antialiased text than > they were in dillo1, so we should at least try to reduce them. I agree, and it looks like the right thing to do. > > > The only rather simple improvement I can think of is that we could > > > use the image sizes provided in the <img/> tags and only issue a > > > Layout::queueResize() if the image proves to be of a different size. > > > Currently there is a Layout::queueResize() for every image as it > > > gets loaded. > > > > A simple way to throttle redraws is to make it in the html > > parser, and parse at let's say at 3 seconds intervals and at EOF. > > I've done that before on prototypes. > > Yes, but I actually like dillos behaviour to show incoming content > immediately in contrast to firefox and friends. Me too. ;) -- Cheers Jorge.-

Nov. 9, 2007

[patch] make partial redraw requests work

by Johannes.Hofmann＠gmx.de

Hi, this patch adds set(), isEmpty(), and merge() methods to core::Rectangle. These methods are then used to maintain a rectangle that contains all screen areas for which a redraw was requested. When fltk really draws to the screen, the drawing is limited to this area. The most obvious improvement is visible on pages with very large images that take some time to load. While the image is loaded the rest of the page is no longer flickering - at least if there is no other stuff that requests complete redraws. Johannes diff --git a/dw/fltkviewbase.cc b/dw/fltkviewbase.cc --- a/dw/fltkviewbase.cc +++ b/dw/fltkviewbase.cc @@ -42,6 +42,7 @@ FltkViewBase::FltkViewBase (int x, int y canvasWidth = 1; canvasHeight = 1; bgColor = WHITE; + redrawArea.set (0, 0, 0, 0); } FltkViewBase::~FltkViewBase () @@ -50,9 +51,18 @@ FltkViewBase::~FltkViewBase () void FltkViewBase::draw () { - Group::draw (); + int d = damage(); + ::fltk::Rectangle rect (x(), y(), w(), h()); - ::fltk::Rectangle rect (x(), y(), w(), h()); + //Group::draw (); + + if (d == DAMAGE_VALUE) { + ::fltk::push_clip( + translateCanvasXToViewX(redrawArea.x), + translateCanvasYToViewY(redrawArea.y), + redrawArea.width, + redrawArea.height); + } /* fltk-clipping seems not to use widget coordinates */ ::fltk::intersect_with_clip(rect); @@ -68,6 +78,12 @@ void FltkViewBase::draw () area.width = rect.w(); area.height = rect.h(); theLayout->expose (this, &area); + + if (d == DAMAGE_VALUE) { + ::fltk::pop_clip(); + } + + redrawArea.set (0, 0, 0, 0); } core::ButtonState getDwButtonState () @@ -223,9 +239,9 @@ void FltkViewBase::finishDrawing (core:: void FltkViewBase::queueDraw (core::Rectangle *area) { - /** \bug Inefficient */ + redrawArea.merge (area); - redraw (DAMAGE_EXPOSE); + redraw (DAMAGE_VALUE); } void FltkViewBase::queueDrawTotal () diff --git a/dw/fltkviewbase.hh b/dw/fltkviewbase.hh --- a/dw/fltkviewbase.hh +++ b/dw/fltkviewbase.hh @@ -14,6 +14,7 @@ class FltkViewBase: public FltkView, pub { private: int bgColor; + core::Rectangle redrawArea; protected: core::Layout *theLayout; diff --git a/dw/types.cc b/dw/types.cc --- a/dw/types.cc +++ b/dw/types.cc @@ -27,10 +27,7 @@ namespace core { Rectangle::Rectangle (int x, int y, int width, int height) { - this->x = x; - this->y = y; - this->width = width; - this->height = height; + set (x, y, width, height); } /** @@ -111,6 +108,35 @@ bool Rectangle::isPointWithin (int x, in x >= this->x && y >= this->y && x < this->x + width && y < this->y + height; } + +/** + * \brief Enlarge the rectangle so that it also includes otherRect. + * If a rectangle has zero size, it is ignored. + */ +void Rectangle::merge (Rectangle *otherRect) +{ + if (otherRect->isEmpty()) { + return; + } + + if (isEmpty()) { + x = otherRect->x; + y = otherRect->y; + width = otherRect->width; + height = otherRect->height; + return; + } + + width = ::misc::max(x + width, otherRect->x + otherRect->width); + height = ::misc::max(y + height, otherRect->y + otherRect->height); + + x = ::misc::min(x, otherRect->x); + y = ::misc::min(y, otherRect->y); + + width -= x; + height -= y; +} + // ---------------------------------------------------------------------- diff --git a/dw/types.hh b/dw/types.hh --- a/dw/types.hh +++ b/dw/types.hh @@ -68,9 +68,18 @@ public: inline Rectangle () { } Rectangle (int x, int y, int width, int height); + void set (int x, int y, int w, int h) { + this->x = x; + this->y = y; + this->width = w; + this->height = h; + } + bool intersectsWith (Rectangle *otherRect, Rectangle *dest); + void merge (Rectangle *otherRect); bool isSubsetOf (Rectangle *otherRect); bool isPointWithin (int x, int y); + bool isEmpty() {return width == 0 && height == 0;}; }; /**

Nov. 9, 2007

[patch] enable partial image drawing

by Johannes.Hofmann＠gmx.de

Hi, here is a first patch to reduce redrawing in dillo-fltk. It just enables code that already existed but was commented out and allows drawing of parts of an image only. I added some additional checks. With these checks I did not see any negative effects. Cheers, Johannes diff --git a/dw/fltkimgbuf.cc b/dw/fltkimgbuf.cc --- a/dw/fltkimgbuf.cc +++ b/dw/fltkimgbuf.cc @@ -334,7 +334,19 @@ void FltkImgbuf::draw (::fltk::Widget *t // xRoot, x, yRoot, y, width, height, this->width, this->height); //{ -#if 0 +#if 1 + if (x > this->width || y > this->height) { + return; + } + + if (x + width > this->width) { + width = this->width - x; + } + + if (y + height > this->height) { + height = this->height - y; + } + // almost OK for rows. For some unknown reason it trims the bottom and // rightmost parts when scrolling. ::fltk::Rectangle rect (xRoot + x, yRoot + y, width, height);

Nov. 9, 2007

forms related segfault

by jcid＠dillo.org

On Fri, Nov 09, 2007 at 01:15:19PM +0000, place wrote: > > > [Jorge wrote] > > > BTW, I asked on the linkblock/callback patch, because if I look at it, > > > you can devote your time to gzip decoding. > > > > > [place wrote] > > > All right. > > > > :-) > > > > So maybe this means he's expecting me to do it. > > > > Place, would you mind confirm? > > Yes, I thought you were doing it... > > What I had meant about page url and base_url was that I'd started, screwed > it up badly, and happily scrapped it when I saw your message. Good. I'll make it ASAP then and let Johannes know so we can solve the forms problem too. -- Cheers Jorge.-

Nov. 9, 2007