https://issues.apache.org/bugzilla/show_bug.cgi?id=50006

Helder Magalhães <[email protected]> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |PatchAvailable
           Platform|PC                          |All
            Summary|ttf2svg outputs hkern       |[PATCH] ttf2svg outputs
                   |elements outside glyph      |hkern elements outside
                   |range                       |(user-provided) glyph range
         OS/Version|Linux                       |All
           Severity|normal                      |enhancement

--- Comment #1 from Helder Magalhães <[email protected]> 2010-09-27 
01:38:56 EDT ---
(In reply to comment #0)
> Created an attachment (id=26077)
 --> (https://issues.apache.org/bugzilla/attachment.cgi?id=26077) [details]
> Patch to only dump kerning table elements requested by the user
> 
> ttf2svg dumps the entirety of the kerning table instead of the subset in the
> low/high range given by the user.

Sounds like a good idea - I'm an adept of optimizations. :-)


(from the patch)
> -                    ps.println(getKerningPairAsSVG(kst.getKerningPair(i), 
> post));
> +                     int left = kst.getKerningPair(i).getLeft();
> +                     int right = kst.getKerningPair(i).getRight();
> +                     boolean left_in_range = (first <= left && left <= last);
> +                     boolean right_in_range = (first <= right && right <= 
> last);
> +                     if (left_in_range && right_in_range) {
> +                     ps.println(getKerningPairAsSVG(kst.getKerningPair(i), 
> post));
> +                    }


Should we optimize things a bit, maybe combining the comparison
verbosity/additional variable creation into a comment? Something like:

+                    int left = kst.getKerningPair(i).getLeft();
+                    int right = kst.getKerningPair(i).getRight();
+                       // check if left and right are both in range
+                    if ((first <= left && left <= last) && (first <= right &&
right <= last)) {


Also, the patch is using tabs for indentation. Please also replace with spaces
to match Batik's code standards. Other than that, looks good. ;-)


I've adjusted the bug's properties to reflect a patch being available, as well
as other minor stuff.


Thomas, what do you thing?

-- 
Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to