Hi Vincent, In the last commit to Temp_RoundedCorners I have addressed all your points, except: > • o.a.f.render.intermediate.BorderPainterTestCase > the generics don’t seem necessary and could be removed altogether I would like to improve that little micro-framework to eliminate the need for generics (and the cast I had to do!) but I think it serves it's purpose for now.
Thanks, Pete On Fri, Oct 19, 2012 at 4:43 PM, Vincent Hennebert <[email protected]> wrote: > +1 > > Having the ‘absolute’ versions of the properties would be nice but this > is not IMO a blocker requirement for the merge of this work. > > Here are a few comments based on a quick look at the branch: > • o.a.f.fo.Constants > PR_X_BORDER_START_RADIUS_START > PR_X_BORDER_START_RADIUS_END > PR_X_BORDER_END_RADIUS_START > PR_X_BORDER_END_RADIUS_END > Shouldn’t it be > PR_X_BORDER_START_RADIUS_BEFORE > PR_X_BORDER_START_RADIUS_AFTER > PR_X_BORDER_END_RADIUS_BEFORE > PR_X_BORDER_END_RADIUS_AFTER > ? > • o.a.f.fo.properties.BoxCornerPropShorthandParser > some glitches in the javadoc for convertValueForProperty > • o.a.f.fo.properties.CommonBorderPaddingBackground > there’s a commented out ‘if (style != Constants.EN_NONE) {’? Is that > left-overs from debugging that can be removed? > • o.a.f.layoutmgr.TraitSetter > line ~100: it’s a shame to re-introduce Hungarian notation > • o.a.f.render.intermediate.BorderPainter > left-over ‘TODO remove before integration’? > drawBorderSegment: don’t you want to use Math.round instead of > directly casting to int, for better precision? > • o.a.f.render.intermediate.BorderPainterTestCase > the generics don’t seem necessary and could be removed altogether > • o.a.f.render.pdf.PDFPainterTestCase, > o.a.f.render.ps.PSPainterTestCase > the ‘// mock’ comments are hardly helpful and could be removed; the > variables could instead be renamed into something that starts with > ‘mock’ so that we know what is what later in the code. > The try...catch should really be removed: if an exception occurs > during the test it will be swallowed (along with its stack trace) by > the catch and replaced with an unhelpful AssertionError with no stack > trace. > > > Thanks, > Vincent > > > On 12/10/12 10:40, Peter Hancock wrote: >> Hi, >> >> Luis Benardo and Myself have just done some clean up to the branch >> Temp_RoundedCorners. This branch implements support for 'fox' >> extension properties for specifying borders with rounded corners. >> Please refer to [1] and [2] for details. >> >> There is an example fo [3] that demonstrates the feature. >> >> Currently we are supporting: >> PDF, PS and AFP outputs >> 'border-style' property with values of 'solid', 'none', 'hidden' >> and, to a limited degree, 'dashed' >> >> I would like to start a vote to merge feature branch to trunk, with my +1. >> >> Thanks, >> >> Peter >> >> [1] http://wiki.apache.org/xmlgraphics-fop/RoundedBorders >> [2] http://xmlgraphics.staging.apache.org/fop/trunk/extensions.html >> [2] examples/fo/advanced/rounded-corners.fo in the Temp_RoundedCorners branch >>
