giddie / poppler-cairo-backend

A patchset to enable subpixel rendering of fonts via Cairo in Poppler's Qt wrapper
18 stars 2 forks source link

Overly intense fringes #7

Open 0xBRM opened 8 years ago

0xBRM commented 8 years ago

Screenshot comparison Look at the t's, the i's and the p's. I know it is required but it seems a bit excessive.

Zoomed in is nowhere near as bad.

Edit: for some reason it seems to alter Okular's thumbnail generation as well, and not simply the text based ones. Edit2: I am using poppler 0.42.0.

giddie commented 8 years ago

Yeah, I'm not happy with the fringes either. I imagine there's some filtering that could be applied to this, but I'm afraid I know pretty much nothing about how Cairo does subpixel rendering. Patches or research would be greatly appreciated. A good starting point might be to look for any patches people have made to apply subpixel rendering in Evince, as we should be able to apply the same code in this patchset.

It doesn't surprise me too much that you're seeing differences in thumbnail generation in Okular: this patchset changes the whole rendering engine that Okular uses to match that of Evince and any other apps that use the Cairo backend.

0xBRM commented 8 years ago

Okay, so the problem is that Cairo, as used by poppler, will default to FT_LCD_FILTER_LEGACY and this is what's causing all the issues. When you force it to use FT_LCD_FILTER_DEFAULT (where the default is "08 24 36 24 08", as per infinality/fontconfig-ultimate's patchset, though you can try using vanilla freetype) the results look much better.

FT_LCD_FILTER_LEGACY

FT_LCD_FILTER_DEFAULT

The overly bold text can be remedied by using a more up-to-date pixman, or by including a slightly lighter default LCD filter (LCD_FILTER_LIGHT).

giddie commented 8 years ago

So do you know what cairo method call is required to apply the filter? I'm hoping it can be easily added in the Cairo font options in CairoOutputDev::setCairo().

0xBRM commented 8 years ago

It used to be cairo_font_options_set_lcd_filter() but they have since removed it, so I'm using a patched Cairo.

Screenshot comparison

As you can see it looks noticeably better, without the fringes. Smaller fonts look amazing as well.

zhou13 commented 8 years ago

You may also interest in my patchset https://github.com/zhou13/poppler-subpixel, which only enables subpixel rendering when the correctness is guaranteed by calling poppler_page_support_subpixel_rendering.

0xBRM commented 8 years ago

@zhou13 Ah, I actually used your patchset. I'm going to try your patch and report back with results.

Redacted due to misunderstanding.

zhou13 commented 8 years ago

What I notice is that zhou13's version in you screenshot does not has subpixel rendering at all... Directly applying my patchset won't change the result of okular because it is designed for evince now. In order to make it work for okular, you need to patch okular also (to explicit enable subpixel rendering). As for the result, I expect they should be exactly the same on most PDF files unless blending mode are used.

0xBRM commented 8 years ago

@zhou13 Oh, I see, so it doesn't add support for the cairo backend to the Qt4 bindings. My bad. Should have read the patchset.

giddie commented 8 years ago

Thanks @zhou13; you've done the Cairo-backend-side work to support subpixel-rendering properly, which is what I was hoping for. It's unfortunate that it requires a patched Cairo. If that weren't the case, I'd certainly try to include your work in this patchset too, as the current patch I use to make the Cairo backend do subpixel-rendering is pretty basic.

Do you know whether Cairo is interested in merging the necessary code upstream?

zhou13 commented 8 years ago

https://bugs.freedesktop.org/show_bug.cgi?id=10301

It seems that comments in bugzilla are ignored as usual. Maybe we need to submit it to the developer mail list.