Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CSS box-shadow and text-shadow #13

Open
SimonSapin opened this issue Oct 3, 2012 · 13 comments
Open

CSS box-shadow and text-shadow #13

SimonSapin opened this issue Oct 3, 2012 · 13 comments
Labels
feature New feature that should be supported

Comments

@SimonSapin
Copy link
Member

WeasyPrint currently does not support the CSS box-shadow or text-shadow properties.

The main limitation is that these shadow use a Gaussian blur, but cairo (by itself) does not do such blurs. Since the blur can not be expressed as vectors in a PDF file, they need to be rendered to pixel-based images. It would be best to make images at the surface’s fallback resolution. Scipy can do the gaussian filter on a numpy array based on the image’s data.

@SimonSapin
Copy link
Member Author

https://code.google.com/p/cairocks/ has a gaussian filter for cairo image surfaces.

@SimonSapin
Copy link
Member Author

#cairo @ Freenode

mrobinson: As far as I know, every standard says something along the lines of "do something like a gaussian."
mrobinson: In WebKit we use three box blurs.

@liZe
Copy link
Member

liZe commented Dec 27, 2013

box-shadow has been implemented in the "shadow" branch.

Good news:

  • No regressions in tests
  • Multiple inset/outset shadows supported
  • Almost no impact on speed for boxes with no box-shadow
  • Many tests for box-shadow property syntax
  • Quite good results on real-world tests
  • 2 out of 2 (only!) tests in W3C test suite pass, including the one testing the blur rendering (yes, that's a real gaussian blur 😄)
  • Not so bad performance for small boxes with no blur or small blur

Bad news:

  • Current implementation relies on pillow (well, that's better than scipy)
  • Raster (that can be avoided for box-shadows with no blur)
  • Really poor performance for large boxes with large blur, can easily kill your computer (yes, that's a real gaussian blur 😢)
  • No tests for rendering (easy to add for box-shadow without blur, more difficult with blur)

It's possible to implement a three-box-blur algorithm instead of the real gaussian blur of pillow. I'm not sure about the impact on speed (who said that it should be included in cairocffi?), but it's a good idea if we want to get rid of pillow (is that what we want?). We could also remove this ugly cairo-surface to pillow-image to cairo-surface translation.

If the code is good for everybody, we can merge and add text-shadow (anyone interested?).

@liZe
Copy link
Member

liZe commented Dec 27, 2013

Rendering tests added (except for blurs), the code looks really reliable now.

@liZe
Copy link
Member

liZe commented Dec 27, 2013

text-shadow added with tests for syntax.

@liZe
Copy link
Member

liZe commented Oct 22, 2014

@SimonSapin
Copy link
Member Author

Interesting… now let’s see if and when it materializes :]

@SimonSapin
Copy link
Member Author

Pom pom pom. https://pythonhosted.org/azureblur/

@SimonSapin
Copy link
Member Author

Also, as nice as they are on screen, I’m told shadows look terrible on print since you have to dither to do grays.

@Afoucaul
Copy link

Afoucaul commented Apr 27, 2019

Hi there, I commented on the related PR but I guess it's better suited to post here.

Do you guys still plan on implementing the box-shadow property?
I would have loved to see @liZe's implementation from shadow merged into master, to have at least one implementation upon which one could have improved.
I'm currently trying to merge that branch into master, but it's not trivial for me as I don't really know the codebase.

Do you know, as of today, what would be a satisfying implementation for this?
I'm gonna try and make @liZe's implementation usable with today's code.
If this works out well, I'm intending to implement the property in a more efficient and robust way.

Anyway, thanks for all your great work!

@liZe
Copy link
Member

liZe commented May 15, 2019

Hello @Afoucaul.

First of all, thank you for your pull request #859 and for the improvements you want to include in WeasyPrint.

Do you guys still plan on implementing the box-shadow property?

Yes, we do.

I would have loved to see @liZe's implementation from shadow merged into master, to have at least one implementation upon which one could have improved.
I'm currently trying to merge that branch into master, but it's not trivial for me as I don't really know the codebase.

Well…

Shadows can be useful, even if they're not great when printed. The problem we have here is that the only solutions we've found are based on raster images, and raster images are bad: they're heavy, they don't scale, they're ugly when printed.

The only possible solution for now would be to have shadows in Cairo. But we don't. And that's sad.

Moreover, PIL is a huge dependency. We're currently trying to have less dependencies (especially the ones with C code), because they're a pain to install for users (see #841). And Azureblur was a proof of concept, it's never been uploaded to PyPI.

Another solution would be to find a replacement for Cairo that handles shadows (see #342 for example).

So… For now, I'll close your pull request, as it relies on both PIL and Azureblur. Even with only PIL, I wouldn't merge it (no offense, I even rejected my own pull request 😉). I really prefer clean solutions now, as I've had to maintain and deal with a lot of wrong choices before, and it's not funny at all. I know it can be frustrating, but I'm sure that it will force us to find even better solutions.

@Afoucaul
Copy link

Afoucaul commented May 15, 2019

Hello @liZe, thanks for your feedback!

I'll close your pull request, as it relies on both PIL and Azureblur

Actually this does not need PIL anymore. I kept it in the first place because I integrated your work that used it, but now Azureblur is the only dependency of my implementation.
I might have let some imports but these would be omissions.

And Azureblur was a proof of concept, it's never been uploaded to PyPI.

But it's straightforward to use, and works out of the box. Seriously, I cloned it, python setup.py installed it, and it just worked. To me, it seems perfect as it is, but do you see any features it lacks, or any fixes it needs?

I really prefer clean solutions now, as I've had to maintain and deal with a lot of wrong choices before, and it's not funny at all. I know it can be frustrating, but I'm sure that it will force us to find even better solutions.

I strongly agree with your position about keeping things clean. However, reading the state of affairs about shadows in Cairo, it looks like there's never going to be a standard solution, and that everyone out there has resigned themselves to implementing their own way.
Based upon this observation, I believe that the simplest approach is using a well-maintained third-party library, that leads to clean code on the Python side.
That's what I see in Azureblur: the C lib is widely used, and the Python bindings just work and are very intuitive to use.

I will not however try to force a personal decision upon you, the choice is yours.
But if you find out an approach that entirely satisfies you, please let me know, and I'd be glad to help you implement it 😁

@liZe
Copy link
Member

liZe commented May 29, 2019

I might have let some imports but these would be omissions.

Yes, it's only omissions. No problem, I could fix this issue from your PR.

But it's straightforward to use, and works out of the box. Seriously, I cloned it, python setup.py installed it, and it just worked. To me, it seems perfect as it is, but do you see any features it lacks, or any fixes it needs?

Cloning a repository to install WeasyPrint is OK for you, but it's definitely not a solution for most of our users 😄.

There are many really big problems with Azureblur:

  • It's not even released on PyPI.
  • It's not maintained.
  • It's not tested.
  • It bundles its own (outdated) versions of Moz2D.
  • It needs a C / C++ compiler to be built.

However, reading the state of affairs about shadows in Cairo, it looks like there's never going to be a standard solution, and that everyone out there has resigned themselves to implementing their own way.

Adding blur in a way that's adapted to the many output backends Cairo supports is not a trivial task. As I've said before, generating a raster image for a PDF is a bad solution for me, no matter how good and how simple the code is to do this. I've even rejected my own pull request.

The worst PR I could merge is a raster shadow with no extra libraries (and it really has to be really simple). You may be able to do this by adapting the official blur snippet for Cairo into a Python function using CairoCFFI.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature that should be supported
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants