[Dillo-dev] Good news in dillo's rendering!

9 views
Skip to first unread message

Jorge Arellano Cid

unread,
Jul 13, 2016, 1:40:32 PM7/13/16
to Dillo mailing list
Hi there,

Here go some good news.

Originally I was to send this internally to the core devs, but
as we've benefitted from the feedback of active users in the
list, and their experiences with the patches (before/afterwards),
on the sites they usually browse, it looks like a good idea to do
it again.

Since June 6, when this thread [1] was started by Sebastian,
I've been investigating/working in problems related to it. This
led to some extensive rendering tests that catched a laundry list
of problems with our current tip (#4643 today).

This is a list with brief explanations, taken from the testing
data tables I've gathered so far:

Problems:
1: overwrites text with images.
2: crops images (aspect ratio).
3: different layout.
4: overwrites text with text float.
5: overwrites text with text.
6: CPU HOG with: tmp.num.html, tmp.num.small.html
7: Missing page sections (sometimes works for the same page).
8: Small images cut in two sometimes ("ESP", "CRO", ...)
9: Wrong aspect ratio for images (text line height)
10: Page drawn with offset (but browser thinks other section is
on screen).

There're test cases for each one, and most of them were taken
and reduced from real pages.

Having reduced test cases is good, reduced and isolated tests
is better, but the good news is that I have a couple of patches
that solve all of them!

I'm not saying dillo will be error-free hereafter, but that the
root of a whole family of problems has been found and fixed, and
this will make future testing much simpler.

GROWS2 is the "Grand Redesign Of Widget Sizes" v2 by Sebastian.
This is a very powerful and flexible concept whose design allows
floats, layers (and absolutes in the future, and more). It has
proven very fast so far, even without optimizations.

Well the patches are two simple but deep changes in the way
markSizeChange() and queueResize() interact. The patch idea is
close to graph coverage recursions (with pruning), applied to the
widget tree.

I'd like to commit it, upload the test suite I'm using, and
show the testing data tables I've gathered so everyone can check
it with their use cases and report on it.

@Sebastian: May I, or would you prefer to review them before?
(the patches are the "middle ground" you mentioned. Solve the dw
hog problems at dw level, not in the parser, and reuse code
you've already seen. A couple of days should be enough to review)


@Sebastian: Problem #10 is not related to GROWS2, but a three
year old bug discovered during the tests. So let's get it done
first. How to reproduce it: Go to [2], wait a bit for most images
to load, then use "find text" to find the "re-re" string, scroll
a bit downwards to find the image and center it. Go back then
forward; no image! But dillo thinks it *is* there so move the
cursor to the center of the page and you'll see the hand cursor,
wait for the tooltip and drag it a bit downwards to the right.
Dillo will repaint from the image data.
Bisect shows the bug was introduced by #2863. Attached is a
simple concept patch for it. Please review/fix/commit, as I don't
know the updated invariants of resize idle.


[1] http://lists.dillo.org/pipermail/dillo-dev/2016-June/010887.html
[2] http://tinyurl.com/huvf6pn


--
Cheers
Jorge.-

_______________________________________________
Dillo-dev mailing list
Dill...@dillo.org
http://lists.dillo.org/cgi-bin/mailman/listinfo/dillo-dev

Jorge Arellano Cid

unread,
Jul 13, 2016, 3:29:50 PM7/13/16
to Dillo mailing list
On Wed, Jul 13, 2016 at 01:37:12PM -0400, Jorge Arellano Cid wrote:
> [...]

> @Sebastian: Problem #10 is not related to GROWS2, but a three
> year old bug discovered during the tests. So let's get it done
> first. How to reproduce it: Go to [2], wait a bit for most images
> to load, then use "find text" to find the "re-re" string, scroll
> a bit downwards to find the image and center it. Go back then
> forward; no image! But dillo thinks it *is* there so move the
> cursor to the center of the page and you'll see the hand cursor,
> wait for the tooltip and drag it a bit downwards to the right.
> Dillo will repaint from the image data.
> Bisect shows the bug was introduced by #2863. Attached is a
> simple concept patch for it. Please review/fix/commit, as I don't
> know the updated invariants of resize idle.
>
>
> [1] http://lists.dillo.org/pipermail/dillo-dev/2016-June/010887.html
> [2] http://tinyurl.com/huvf6pn

As usual, the attachment arrives later... ;)

--
Cheers
Jorge.-
layout.diff

Jorge Arellano Cid

unread,
Jul 16, 2016, 11:02:39 PM7/16/16
to Dillo mailing list
Hi there,

OK, the patches were just committed, there're four instead of
two and the last one has the full test suite results inside the
comments in Hg.

These patches work very well so far.

The idea is to get your testing and feedback.

I still have some ideas and cleanups for them, but that will take
more time, and this set is already working and very fast.

Enjoy!

PD: a bit overworked Jorge. :-P

Sebastian Geerken

unread,
Jul 17, 2016, 8:07:30 AM7/17/16
to dill...@dillo.org
On Mi, Jul 13, 2016, Jorge Arellano Cid wrote:
> GROWS2 is the "Grand Redesign Of Widget Sizes" v2 by Sebastian.
> This is a very powerful and flexible concept whose design allows
> floats, layers (and absolutes in the future, and more). It has
> proven very fast so far, even without optimizations.

It's actually called "SRDOP" ("Size Requisitions Depending On
Positions). See

- http://www.dillo.org/dw/html/dw-grows.html
- http://www.dillo.org/dw/html/dw-size-request-pos.html

As to current bugs, most of them fall into either one category:

1. missing rendering updates: this results in rendering errors (wrong
positions etc.);

2. inefficient (too many) updates: this results in CPU hogging.

You see that these are complementary. :-)

The focus should now be on the first category, and I've still at least
one open issue here, which is neither addressed by Jorge's patches.
Problems from the second category are best fixed after this.

Sebastian


signature.asc

Jorge Arellano Cid

unread,
Aug 4, 2016, 10:55:59 AM8/4/16
to Dillo mailing list

Reverting reverts:
-----------------

There were a few unfortunate commits that break the work
detailed in this thread, so basically we're getting back to
r4648, keeping the patches that don't make regressions in this
test suite [1], by reverting #4651 and #4652, and putting #4653
momentarily "on hold".

In a nutshell:

Results Table
.-------.--------.------.-----------.-----------------------.-----.-----.
| | BTG | test | pravda.ru | 4ta.html | VT | PV |
| | render | t.* | #1 | #2 | Girl | footb. | horos.| | |
|-------|--------|------|-----------|-----------------------|-----|-----|
|d4657 | ok | ok | ok | ok | ok | ok | ok | ok | ok |
|d4652 | ok | ok | x a | x ic| ok | x l | x d | x n | ok |
|d4651 | ok | ok | x a | x ic| ok | x l | x d | x n | ok |
|d4648 | ok | ok | ok | ok | ok | ok | ok | ok | ok |
'-----------------------------------'-----------------------------'-----'

As you can see, d4648 and d4657 pass all the tests.

Technical details in "hg log -v" and [1].

Hereafter we'll use this testsuite [1] to assert committed code
at least passes this known bugset.

Besides the test suite, I did extensive tests with the sites I
usually visit, with no regressions (BTW, a three year old bug or
so was catched in the process).


This was meant to be a moment of joy for dillo users, so please
give these patches extensive tests/surfing and report back how
they work for you! (good and bad).

Hint: I found a near 100% speed increase with a heavy site I
use (Back/Forward test).

[1] http://www.dillo.org/test/4648/test-suite.v1.txt

Reply all
Reply to author
Forward
0 new messages