Re: [Libreoffice] [PATCH] fdo#31251 Writer & Impress shadow consistency
Le Tue, 3 May 2011 14:23:44 +0200,
David Tardon a écrit :
> On Sat, Apr 30, 2011 at 06:58:29PM +0200, Sébastien Le Ray wrote:
> > if(aShadowColor != SwViewOption::GetShadowColor() ) {
> > aShadowColor = SwViewOption::GetShadowColor();
> > -AlphaMask aMask( SW_RES( BMP_PAGE_BOTTOM_RIGHT_SHADOW_MASK
> > ) );
> > -Bitmap aFilledSquare( Size( mnShadowPxWidth,
> > mnShadowPxWidth ), 24 );
> > -aFilledSquare.Erase( aShadowColor );
> >
> > +AlphaMask aMask( shadowMask.getBottomRight().GetBitmap() );
> > +Bitmap aFilledSquare( aMask.GetSizePixel(), 24 );
> > +aFilledSquare.Erase( aShadowColor );
> > ...
>
> This looks like a good candidate for a function... Like
>
> void lcl_createShadow(Color const& rColor, BitmapEx const& rMask,
> BitmapEx& rShadow)
> {
> AlphaMask aMask( rMask.GetBitmap() );
> Bitmap aFilledSquare( aMask.GetSizePixel(), 24 );
> aFilledSquare.Erase( rColor );
> rShadow = aFilledSquare;
> }
>
> But this is really just "nice to have", not a blocker :)
OK, I'll try to find some time to write it
> [...]
> Do I count wrong or is the right shadow painted over the bottom right
> corner shadow? And the same for left shadow lower.
Well, it shouldn't since we do not have visual overlap. Corners are
17px large, 8 of them come to the adjacent borders so we scale borders
to remove those 8px…
>
> > @@ -5326,20 +5326,19 @@ const sal_Int8 SwPageFrm::mnShadowPxWidth =
> > 9; aPageTopRightShadow );
> > BitmapEx aPageRightShadow = aPageRightShadowBase;
> > aPageRightShadow.Scale( 1, aPagePxRect.Height() - 2 *
> > (mnShadowPxWidth - 1) );
> > -
> > pOut->DrawBitmapEx( pOut->PixelToLogic( Point( aPagePxRect.Right()
> > + 1, aPagePxRect.Top() + mnShadowPxWidth - 1) ), aPageRightShadow );
> > +
> > pOut->DrawBitmapEx( pOut->PixelToLogic( Point( aPaintRect.Right() +
> > mnShadowPxWidth, aPagePxRect.Top() + mnShadowPxWidth - 1) ),
> > aPageRightShadow );
>
> Is this really correct? AFAICS it moves the shadow too much to the
> right... And if it is correct, why the corner shadows do not need to
> be adjusted too?
Still because of the fact we've the 8px overlapping the adjacent
border. I guess this picture
http://misc.orniz.org/libreoffice/shadow-decomposition.png should make
things clearer.
As you can see, Paint rect is only used for corners shadow left/right
adjustment
Sébastien
signature.asc
Description: PGP signature
___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: [Libreoffice] [PATCH] fdo#31251 Writer & Impress shadow consistency
On Tue, May 03, 2011 at 11:55:38AM +0200, Jan Holesovsky wrote: > Hi Sebastien, > > On 2011-04-30 at 18:58 +0200, Sébastien Le Ray wrote: > > > Since OOo UX team implemented a new shadow on impress which seem to > > please more people than the writer shadow I provided before, here is a > > patchset that add the same shadow to writer. > > So - I like it, and would love to include for 3.4; one more review > needed :-) - David? Cedric? +1 from me, even with the small glitches > I have noticed one thing that I think is still worth fixing before the > release - unlike in Impress, there is no black line around the page, > see: > > http://artax.karlin.mff.cuni.cz/~kendy/libreoffice/missing-black-line.png > > [Writer on the left, Impress on the right] > > This causes the mode when you have 2 pages next to each other look a bit > strange: > > http://artax.karlin.mff.cuni.cz/~kendy/libreoffice/no-division-between-pages.png This is actually because, in book mode, there is a small overlap between the pages--it is clearly visible with just two pages. I guess it is caused by the aSpaceToNextPage/aSpaceToPrevPage adjustments in SwLayAction::FormatLayout . D. ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: [Libreoffice] [PATCH] fdo#31251 Writer & Impress shadow consistency
On Sat, Apr 30, 2011 at 06:58:29PM +0200, Sébastien Le Ray wrote:
> if(aShadowColor != SwViewOption::GetShadowColor() ) {
> aShadowColor = SwViewOption::GetShadowColor();
> -AlphaMask aMask( SW_RES( BMP_PAGE_BOTTOM_RIGHT_SHADOW_MASK ) );
> -Bitmap aFilledSquare( Size( mnShadowPxWidth, mnShadowPxWidth ), 24 );
> -aFilledSquare.Erase( aShadowColor );
>
> +AlphaMask aMask( shadowMask.getBottomRight().GetBitmap() );
> +Bitmap aFilledSquare( aMask.GetSizePixel(), 24 );
> +aFilledSquare.Erase( aShadowColor );
> ...
This looks like a good candidate for a function... Like
void lcl_createShadow(Color const& rColor, BitmapEx const& rMask,
BitmapEx& rShadow)
{
AlphaMask aMask( rMask.GetBitmap() );
Bitmap aFilledSquare( aMask.GetSizePixel(), 24 );
aFilledSquare.Erase( rColor );
rShadow = aFilledSquare;
}
But this is really just "nice to have", not a blocker :)
> if ( bPaintRightShadow )
> {
> -SwPageFrm::GetRightShadowRect( _rPageRect, _pViewShell, aPaintRect,
> bRightSidebar );
> +pOut->DrawBitmapEx( pOut->PixelToLogic( Point( aPaintRect.Right() +
> 1, aPagePxRect.Bottom() + 1 - (aPageBottomRightShadow.GetSizePixel().Height()
> - mnShadowPxWidth) ) ),
> +aPageBottomRightShadow );
> +pOut->DrawBitmapEx( pOut->PixelToLogic( Point( aPaintRect.Right() +
> 1, aPagePxRect.Top() - mnShadowPxWidth ) ),
> +aPageTopRightShadow );
> BitmapEx aPageRightShadow = aPageRightShadowBase;
> -aPageRightShadow.Scale( 1, aPaintRect.Height() );
> -pOut->DrawBitmapEx( pOut->PixelToLogic( aPaintRect.Pos() ),
> aPageRightShadow );
> -pOut->DrawBitmapEx( pOut->PixelToLogic( Point( aPaintRect.Left(),
> aPaintRect.Top() - mnShadowPxWidth ) ), aPageTopRightShadow );
> -pOut->DrawBitmapEx( pOut->PixelToLogic( aPaintRect.BottomLeft() ),
> aPageBottomRightShadow );
> +aPageRightShadow.Scale( 1, aPagePxRect.Height() - 2 *
> (mnShadowPxWidth - 1) );
> +pOut->DrawBitmapEx( pOut->PixelToLogic( Point( aPagePxRect.Right() +
> 1, aPagePxRect.Top() + mnShadowPxWidth - 1) ), aPageRightShadow );
Do I count wrong or is the right shadow painted over the bottom right
corner shadow? And the same for left shadow lower.
> @@ -5326,20 +5326,19 @@ const sal_Int8 SwPageFrm::mnShadowPxWidth = 9;
> aPageTopRightShadow );
> BitmapEx aPageRightShadow = aPageRightShadowBase;
> aPageRightShadow.Scale( 1, aPagePxRect.Height() - 2 *
> (mnShadowPxWidth - 1) );
> -pOut->DrawBitmapEx( pOut->PixelToLogic( Point( aPagePxRect.Right() +
> 1, aPagePxRect.Top() + mnShadowPxWidth - 1) ), aPageRightShadow );
> +pOut->DrawBitmapEx( pOut->PixelToLogic( Point( aPaintRect.Right() +
> mnShadowPxWidth, aPagePxRect.Top() + mnShadowPxWidth - 1) ), aPageRightShadow
> );
Is this really correct? AFAICS it moves the shadow too much to the
right... And if it is correct, why the corner shadows do not need to be
adjusted too?
D.
___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: [Libreoffice] [PATCH] fdo#31251 Writer & Impress shadow consistency
Hi Sebastien, On 2011-04-30 at 18:58 +0200, Sébastien Le Ray wrote: > Since OOo UX team implemented a new shadow on impress which seem to > please more people than the writer shadow I provided before, here is a > patchset that add the same shadow to writer. So - I like it, and would love to include for 3.4; one more review needed :-) - David? Cedric? I have noticed one thing that I think is still worth fixing before the release - unlike in Impress, there is no black line around the page, see: http://artax.karlin.mff.cuni.cz/~kendy/libreoffice/missing-black-line.png [Writer on the left, Impress on the right] This causes the mode when you have 2 pages next to each other look a bit strange: http://artax.karlin.mff.cuni.cz/~kendy/libreoffice/no-division-between-pages.png This is not a regression against what we have in the libreoffice-3-4 branch though, so I think we should apply your patches still for the 3.4 anyway. Thank you very much for them! Regards, Kendy ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: [Libreoffice] [PATCH] fdo#31251 Writer & Impress shadow consistency
Hi Sebastien, On 2011-04-30 at 18:58 +0200, Sébastien Le Ray wrote: > Since OOo UX team implemented a new shadow on impress which seem to > please more people than the writer shadow I provided before, here is a > patchset that add the same shadow to writer. Cool stuff :-) So, if nobody objects, I'd still like to apply this to libreoffice-3-4 branch; as it is a feature, it needs 2 reviews - David, anyone, can you please have a look? I'll do my detailed review tomorrow. Thank you for that, Kendy ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
