3 is an arbitrary width, so -1 for this change

I'm also not really convinced about he color, but that's less contentious than having a random 3 in there.

In D20351#445664, @aacid wrote:

3 is an arbitrary width, so -1 for this change

I'm not a huge fan either, but not because it's arbitrary (everything visual is arbitrary) but rather because this makes it look too thick to my eye.

In D20351#445682, @ngraham wrote:

In D20351#445664, @aacid wrote:

3 is an arbitrary width, so -1 for this change

I'm not a huge fan either, but not because it's arbitrary (everything visual is arbitrary) but rather because this makes it look too thick to my eye.

There is issue with the preview rectangle. May be its just me or may be its for everyone. The rectangle has right and bottom sides thicker than the top and the left sides. Sometimes, top and sometime left even disappears. If we make the width >= 2 the top and left side doesn't disappear atleast.

In D20351#445808, @ahmedbilal wrote:

In D20351#445682, @ngraham wrote:

In D20351#445664, @aacid wrote:

3 is an arbitrary width, so -1 for this change

I'm not a huge fan either, but not because it's arbitrary (everything visual is arbitrary) but rather because this makes it look too thick to my eye.

There is issue with the preview rectangle. May be its just me or may be its for everyone. The rectangle has right and bottom sides thicker than the top and the left sides. Sometimes, top and sometime left even disappears. If we make the width >= 2 the top and left side doesn't disappear atleast.

Then it seems like the chosen value of 3 is just working around a deeper issue. In such cases, it's universally preferred to investigate and fix that issue rather than work around it.

I like the color part of this patch.

As for the line width: As mentioned, '3' is arbitrary. You need to find a width that takes current dpi into account. Ideally, the width should be what later gets written into the pdf file.

The fact that the rectangle is currently not completely drawn sounds like a Qt bug. What is the pen width without your patch? Even if it has width 0 it should paint a 1-pixel line. Or is the page downscaled somewhere after rendering?

In D20351#446498, @sander wrote:

I like the color part of this patch.

As for the line width: As mentioned, '3' is arbitrary. You need to find a width that takes current dpi into account. Ideally, the width should be what later gets written into the pdf file.

The fact that the rectangle is currently not completely drawn sounds like a Qt bug. What is the pen width without your patch? Even if it has width 0 it should paint a 1-pixel line. Or is the page downscaled somewhere after rendering?

@sander I managed to make it the same size as the resultant rectangle. However, @ngraham and @aacid says that the width (3px) is too much for their eyes. So, I think it should be at max 2 or something like that. By default, the pen width is 0 which is actually 1-pixel wide as you mentioned. I will look into it whether it is downscaled or not. Thanks for such nice and helpful comment.

Consider splitting the color change into a separate patch. That part seems to be uncontroversial.

The problem with the '3' is that its perceived thickness depends on your screen size and resolution. What looks good to you may look bad on somebody else's computer.

In D20351#446620, @sander wrote:

Consider splitting the color change into a separate patch. That part seems to be uncontroversial.

The problem with the '3' is that its perceived thickness depends on your screen size and resolution. What looks good to you may look bad on somebody else's computer.

Got It. I still can't figure out what is making the top and left side lighter :|