Bug 187681 - shape shadows are not handled correctly
Summary: shape shadows are not handled correctly
Status: RESOLVED UNMAINTAINED
Alias: None
Product: koffice
Classification: Applications
Component: flake (show other bugs)
Version: 2.0
Platform: Compiled Sources Unspecified
: NOR normal
Target Milestone: ---
Assignee: KOffice Bug Wranglers
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-03-20 08:27 UTC by Thomas Zander
Modified: 2015-02-06 13:23 UTC (History)
2 users (show)

See Also:
Latest Commit:
Version Fixed In:


Attachments
Screenshot showing the issues pointed out in the report. (16.74 KB, image/png)
2009-03-20 08:27 UTC, Thomas Zander
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Thomas Zander 2009-03-20 08:27:03 UTC
Version:            (using Devel)
Installed from:    Compiled sources

Shadows are done incorrect.
* A shape that is transparent but has a border gets its border drawn inside the object. (it should be clipped to outside the shape) The text shape shows this.
* You get overlapping shapes painting their shadows on top of each other. I expect the border to be below all shapes. See the green shape being covered by a shadow. It should not be.
* A shape with content that has an alpha channel becomes unreadable because the shadow shows through. See the yellow shape.

See the textshape in the screenshot;
* A shape that has a transparent fill doesn't have a shadow. (it should)
* A shape without a border gets a one-line border drawn anyway. It shouldn't.
Comment 1 Thomas Zander 2009-03-20 08:27:57 UTC
Created attachment 32279 [details]
Screenshot showing the issues pointed out in the report.
Comment 2 Thomas Zander 2009-03-20 08:58:27 UTC
Also;
rotating a shape makes the shadow move.
Rotating a shape also has the effect that the repainting gets buggy and cut off.
Comment 3 Jan Hambrecht 2009-03-22 15:03:57 UTC
SVN commit 942783 by jaham:

fix painting of shadow regarding shape transformation which was broken by the last commit

CCBUG:187681



 M  +35 -14    KoShapeShadow.cpp  


WebSVN link: http://websvn.kde.org/?view=rev&revision=942783
Comment 4 Jan Hambrecht 2009-03-22 15:19:08 UTC
> Shadows are done incorrect.
> * A shape that is transparent but has a border gets its border drawn inside the
> object. (it should be clipped to outside the shape) The text shape shows this.

Why should a transparent shape cover the shadow? After all it is transparent, which means one can see through.

> * You get overlapping shapes painting their shadows on top of each other. I
> expect the border to be below all shapes. See the green shape being covered by
> a shadow. It should not be.

I think it is correct if they have different z-indeces. If their z-index is the same they should not cast a shadow on each other. The latter probably has to be fixed.

> * A shape with content that has an alpha channel becomes unreadable because the
> shadow shows through. See the yellow shape.

Again, why should transparency prevent to see the shadow behind the shape?

> 
> See the textshape in the screenshot;
> * A shape that has a transparent fill doesn't have a shadow. (it should)

In the case of the text shape, no it does not have a transparent fill, it has no fill. If you set a fill, the shadow will be shown.

> * A shape without a border gets a one-line border drawn anyway. It shouldn't.

This was also fixed by my last commit.
Comment 5 Thomas Zander 2009-03-24 23:47:36 UTC
On Sunday 22. March 2009 15.19.10 Jan Hambrecht wrote:
> > * You get overlapping shapes painting their shadows on top of each other.
> > I expect the border to be below all shapes. See the green shape being
> > covered by a shadow. It should not be.
>
> I think it is correct if they have different z-indeces.

I think it depends on how the user views the z-indeces on the objects.
The itemviews people got lots of bugreports when they took this approach and 
basically I think it depends on what your goal is.
If your goal is to illustrate the fact that one shape has a z-index higher than another, then yes.
But we do our best to hide this fact and actually our code assumes that the 
user is not affected by z-indexes much.

Imagine us inserting 2 objects. One after the other.  We place them next to 
each other.  The user never choose which one is over the other and frankly 
doesn't care.  But in our code we force one to be above the other because we auto-assign them indices.
If we then end up casting a shadow of one object on the other that is next to 
it, then that breaks the illusion of them being next to each other.
In effect we are showing an implementation detail.

Maybe we should have a shadow per layer, but I think that its wrong to have one per shape that cast a shadow on another shape.
Comment 6 Jan Hambrecht 2009-03-25 00:22:04 UTC
(In reply to comment #5)
> On Sunday 22. March 2009 15.19.10 Jan Hambrecht wrote:
> > > * You get overlapping shapes painting their shadows on top of each other.
> > > I expect the border to be below all shapes. See the green shape being
> > > covered by a shadow. It should not be.
> >
> > I think it is correct if they have different z-indeces.
> 
> I think it depends on how the user views the z-indeces on the objects.
> The itemviews people got lots of bugreports when they took this approach and 
> basically I think it depends on what your goal is.

Well OOo takes the same approach as we do now, so when changing that you risk getting bugreports from users on the different shadow behaviour in odf files.
Not sure which the preferred bug reports are. ;-)

> If your goal is to illustrate the fact that one shape has a z-index higher than
> another, then yes.
> But we do our best to hide this fact and actually our code assumes that the 
> user is not affected by z-indexes much.

I don't know how you want to hide the concept of the z-index as that is inherently present in our painting code.

> 
> Imagine us inserting 2 objects. One after the other.  We place them next to 
> each other.  The user never choose which one is over the other and frankly 
> doesn't care.  But in our code we force one to be above the other because we
> auto-assign them indices.
> If we then end up casting a shadow of one object on the other that is next to 
> it, then that breaks the illusion of them being next to each other.
> In effect we are showing an implementation detail.

Right it is an illusion, which gets destroyed as soon as you have these two shapes overlap. As i already said, i agree that two shape having the same z-index should not cast a shadow on each other.

> 
> Maybe we should have a shadow per layer, but I think that its wrong to have one
> per shape that cast a shadow on another shape.
Do you mean a shape layer or a z-index-layer" ?
Comment 7 Thomas Zander 2009-03-25 00:43:23 UTC
On Wednesday 25. March 2009 00.22.05 Jan Hambrecht wrote:
> > Imagine us inserting 2 objects. One after the other.  We place them next
> > to each other.  The user never choose which one is over the other and
> > frankly doesn't care.  But in our code we force one to be above the other
> > because we auto-assign them indices.
> > If we then end up casting a shadow of one object on the other that is
> > next to it, then that breaks the illusion of them being next to each
> > other. In effect we are showing an implementation detail.
>
> Right it is an illusion, which gets destroyed as soon as you have these two
> shapes overlap. As i already said, i agree that two shape having the same
> z-index should not cast a shadow on each other.

Well, this kind of illustrates my point as we can not have two shapes with the same z-index since it would be impossible to paint properly.
So its is impossible for the user to get two shapes *not* cast a shadow on each other.
And the reason is purely an implementation detail.

I don't really care that OOo doesn't handle the properly either, there are plenty of other places where shadows are done and the research that Trolltech [1] did shows clearly that a per item shadow as we have now is only optimal from an implementation perspective.

To be honest, I don't see how you can argue that its correct to have this; really. Do the fresh-user test and load a random svg from the net and give everything a shadow. Ask the user if he expects that there are shadows on top of some elements and below another.
I bet the user expects the shadows to be completely behind *all* elements.

The z-indeces are a red-herring. They just don't exist in the minds of anyone outside of the developers.

> > Maybe we should have a shadow per layer, but I think that its wrong to
> > have one per shape that cast a shadow on another shape.
>
> Do you mean a shape layer or a z-index-layer" ?

KoShapeLayer

Or, in other words; the layers that a user explicitly groups shapes in to make them lay above other shapes.

1) http://labs.trolltech.com/blogs/2008/05/30/decoration-items-light-and-shadow-effects/
Comment 8 Thomas Zander 2009-03-25 19:37:57 UTC
[pasting the comment on koffice ML here to keep everything in one place]

On Wednesday 25. March 2009 13:38:21 Steven D'Aprano wrote:
Speaking as a user, I'd expect the result to depend on whether I was 
drawing three-dimensional art with real shadows, or two-dimensional art 
with illusionary shadows. 

In 3D art, I'd expect objects to cast shadows that cover objects lower 
than them. But in 2D art, I'd expect two planes: an object plane, where 
the objects sit, and a shadow plane beneath than. Objects are always 
above every shadow.

*Within* the object plane, objects have ordering, but it's the order of 
(infinitely thin) flat shapes stacked on top of each other. It 
shouldn't affect the shadows beneath them.

That's my two cents worth.

Steven D'Aprano
Comment 9 Christoph Feck 2015-02-06 13:23:17 UTC
Thank you for your bug report or feature suggestion.

The "KOffice" application suite is no longer maintained, and all tickets are now closed.

We recommend to switch to the "Calligra" application suite, which has replacements for all unmaintained KOffice applications:

- KWord was replaced with Calligra Words
- KPlato was replaced with Calligra Plan

For more information, see http://en.wikipedia.org/wiki/Calligra_Suite

(This is an automatic message from the KDE bug triaging team)