Bug 62296

Created attachment 2172 [details]
testcase demonstrating the issue

Konqueror draws small dashes instead of dots around the text.

Created attachment 2173 [details]
testcase as rendered in mozilla

Mozilla gets it right, the border consists of square dots.

Created attachment 4669 [details]
A patch against CVS from today

The problem lies in Qt. Qt draws dotted lines with a width of 1 like dashed
lines. This is for compatibility with Windows.
I wrote a method that draws dotted lines with a width of 1 as they should be.
However, there are two ways of drawing dotted lines. 
Opera draws them O__O__O__O where O is a dot and _ is space.
Mozilla does it this way: O_O_O_O , so only with one space.
CSS says nothing about the way it should be done.
I think it looks better in Opera, so I did it that way. 
Changing it to Mozilla style would be easy, however.

*** Bug 73920 has been marked as a duplicate of this bug. ***

yeah, thanks for the patch, but drawing the pixels individually
is not _that_ fast..

I just had an idea how to workaround Qt's brokenness. let me try
something. 

CVS commit by mueller: 

make single pixel dotted borders render less IE - like.
CCMAIL: 62296-done@bugs.kde.org


  M +27 -1     render_object.cpp   1.250


--- kdelibs/khtml/rendering/render_object.cpp  #1.249:1.250
@@ -550,5 +550,31 @@ void RenderObject::drawBorder(QPainter *
         return;
     case DOTTED:
-        p->setPen(QPen(c, width == 1 ? 0 : width, Qt::DotLine));
+        if ( width == 1 ) {
+            // workaround Qt brokenness
+            p->setPen( QPen( Qt::NoPen ) );
+            p->setBrush( QBrush( c, Qt::Dense4Pattern ) );
+
+            switch(s) {
+            case BSBottom:
+            case BSTop:
+                p->drawRect(x1+1,y1,x2-x1-2,y2-y1);
+                if ( ( x2-x1 ) & 1 ) {
+                    p->setPen( QPen( c, 0 ) );
+                    p->drawPoint( x2-1, y2-1 );
+                }
+                break;
+            case BSRight:
+            case BSLeft:
+                p->drawRect(x1,y1+1,x2-x1,y2-y1-2);
+                if ( ( y2-y1 ) & 1 ) {
+                    p->setPen( QPen( c, 0 ) );
+                    p->drawPoint( x2-1, y2-1 );
+                }
+            }
+
+            break;
+        }
+
+        p->setPen(QPen(c, width, Qt::DotLine));
         /* nobreak; */
     case DASHED:

Created attachment 4695 [details]
A program to test the speed of various line drawing implementations.

Hey, you are fixing faster than your shadow. ;-)
First of all: Where do you all (I saw Stephan Kulow already wrote this in a
comment to the duplicate bug #73920) get the information that pixel drawing is
such a slow operation? Qt docs do not say anything about speed.
And who says that the QBrush with the Dense4Pattern is a fast operation?
(BTW, a hacky trick to draw a dotted line).
So to find it out, I wrote a little test program.
The program draws 60000 lines with a width of 1 and a length of 400 pixels and
measures the time the different implementations need to do this.
I tested 4 implementations:
1. The dotted line with the Qt drawLine function
2. My dotted line implementation
3. An implementation of me to draw a dashed line, that looks exactly like the
Qt dotted line.
4. Your implementation with the Dense4Pattern

The result was a real surprise!
The implementations 2,3 and 4 are all 10 (!) times FASTER than the Qt drawLine
method! So it is not true that the drawPixel method is slow, it is even faster
than the drawLine method (as long as not drawing a solid line).
Also your implementation is as fast as mine, but I like mine better, because it
draws the line in Opera style and yours does it Mozilla like (I described
above). In addition your implementation results in an indetermistic drawing
routine. I have tested your implementation and if you have a test page were you
can scroll down to hide the dotted border and then up again to redraw it, the
dots are drawn randomly on other positions, resulting in not correctly dotted
lines. So please review my patch again.

I have reopened the bug, because at least the dashed line problem is still there.

Created attachment 4699 [details]
testcase rendered in Opera

I very much opt for leaving appearence as determined by Dirk's fix. It
- is totally consistent with mozilla's rendering
- makes focus rectangles on links look the same as in the rest of KDE (compare to list view items' focus rectangles)

I. e. this fix brings khtml nearer to full consistence with the rest of the KDE UI.

Btw, this isn't about the dashed line problem. Please open a bug of its own or undup bug 73920.

I am really sorry to reopen this bug again, but why are you closing it, if it is not fixed correctly?
I do not mind if it looks like Opera or Mozilla, who cares.
The important thing is that the border is drawn in an indetermistic way as I said above already. Also the border does not look good when it is drawn with the Dense4Pattern. I attached three screen shots below. The first one shows the border drawn with the Dense4Pattern, the second one shows the same border, after scrolling down to hide it and scrolling up to show it again. The third screen shot shows the same border drawn by Mozilla.
You will notice that in the first screen shot the border is not correctly drawn. The corners are asymmetric and the top-right corner is missing a dot.
On the second screen shot you will see, that the top border line is missing at all and the other borders are not correctly dotted lines. 

Created attachment 4733 [details]
Border drawn with the Dense4Pattern.

The corners are asymmetric and the top-right corner is missing a dot.

Created attachment 4734 [details]
Same border as above, but after scrolling down and up.

The border looks like a mess.

Created attachment 4735 [details]
Same border as above but rendered by Mozilla.

Ah, sorry. You reopened it because of rendering glitches. I thought you meant border-style:dashed.

Then this was probably my fault by mentioning the dashed border as a reason, sorry for bothering you.

CVS commit by mueller: 

alternative border-style: dotted implementation. Thanks for the performance
tester.

CCMAIL: 62296-done@bugs.kde.org


  M +5 -0      ChangeLog   1.229
  M +5 -6      rendering/render_object.cpp   1.257


--- kdelibs/khtml/ChangeLog  #1.228:1.229
@@ -1,2 +1,7 @@
+2004-03-01  Dirk Mueller  <mueller@kde.org>
+
+        * rendering/render_object.cpp (drawBorder): use drawPixel directly.
+        turns out to be more reliable and equally fast (#62296).
+
 2004-02-29  Dirk Mueller  <mueller@kde.org>
 

--- kdelibs/khtml/rendering/render_object.cpp  #1.256:1.257
@@ -561,17 +561,16 @@ void RenderObject::drawBorder(QPainter *
         if ( width == 1 ) {
             // workaround Qt brokenness
-            p->setPen( QPen( Qt::NoPen ) );
-            p->setBrush( QBrush( c, Qt::Dense4Pattern ) );
-
+            p->setPen(QPen(c, width, Qt::SolidLine));
             switch(s) {
             case BSBottom:
             case BSTop:
-                p->drawRect(x1,y1,x2-x1,y2-y1);
+                for ( ; x1 < x2; x1 += 2 )
+                    p->drawPoint( x1, y1 );
                 break;
             case BSRight:
             case BSLeft:
-                p->drawRect(x1,y1,x2-x1,y2-y1);
+                for ( ; y1 < y2; y1 += 2 )
+                    p->drawPoint( x1, y1 );
             }
-
             break;
         }