You are viewing a plain text version of this content. The canonical link for it is here.
Posted to batik-dev@xmlgraphics.apache.org by Hervé Girod <he...@club-internet.fr> on 2005/08/23 01:04:48 UTC

Re: Enhancements in WMF transcoder

Hi !

Thomas DeWeese wrote:

> Hi Hervé,
>
>> I have enhanced / hacked the actual transcoder code (as of version 
>> 1.6) and added / corrected the following functionalities :
>
>    I see that you already have a CLA on file, so all you need to do
> is build a patch file (svn diff should work well), and attach it to
> a new bugzilla bug.  It would be helpful if the bug comments clearly
> indicates that the patch is donated to Apache. If there are
> new files they should have the Apache header (you can get a copy
> from any file in Batik).
>
OK,  it is done (much more mork than I expected, since I tried to 
"beautify" my code a little). Also I added image / texture handling.

---------------------------------------------------------------------
To unsubscribe, e-mail: batik-dev-unsubscribe@xmlgraphics.apache.org
For additional commands, e-mail: batik-dev-help@xmlgraphics.apache.org


Re: Enhancements in WMF transcoder

Posted by Hervé Girod <he...@club-internet.fr>.
I Thomas,

I modified the files according to your remarks and attached to the RFE 
the new patch. Here are some explanations on the modifications :

Thomas DeWeese wrote:

> XmlWriter changes right now I'm not going to accept due to some issues:
>   1) Your updates removed the use of bulk writes which was
>      specifically added due to performance issues.
>   2) The change to encode all chars >= 0x80 as hex is not friendly
>      for foreign language users....  At most this might be made a option
>      for when writing the document.

  => corrected. I added a parameter in the stream () methods in 
SVGGraphics2D to be able to escape characters only if chosen. The 
default behavior is "not escaped", so it should not break anything.

> SVGGraphics2D
>
> Line 223 in patch:
>   What's with the null paint stuff?  It looks like it isn't used.

=> reverted to Batik reference

> Lines 272 & 292 in patch:
>   I'm not sure that the 'solution' of setting stroke to the
>   'textColor' is a good one

=> reverted to Batik reference

> Line 289:
>   In what case are we missing setting the font-size correctly?
>   Is this for cases where you are using 'getRoot' repeatedly?
>   If so then much more needs to be done I think...

=> reverted to Batik reference

> Line 329:
>   While I appreciate the very good attempt at handling of attributed
>   Character iterators, I am concerned that the code only considers
>   the attributes on the first char. 

=> I now take care of all the runs and create tspans if needed 
(basically, only one text node if only one run, and tspans if there are 
more than one run). I take care of the following attributes : Bold, 
Italic, Underline, Strikethrough, Foreground, Font, Size, Family (more 
than the ones that are needed when transcoding WMF). The new test class 
check the behavior with text decorations / AttributedCharacterIterators.

---------------------------------------------------------------------
To unsubscribe, e-mail: batik-dev-unsubscribe@xmlgraphics.apache.org
For additional commands, e-mail: batik-dev-help@xmlgraphics.apache.org


Re: Enhancements in WMF transcoder

Posted by Thomas DeWeese <Th...@Kodak.com>.
Hi Hervé,

> Thomas DeWeese wrote:
> 
>> XmlWriter changes right now I'm not going to accept due to some issues:
>>   1) Your updates removed the use of bulk writes which was
>>      specifically added due to performance issues.

Hervé Girod wrote:

> Sorry, maybe I first modify this file before this change, and I was a 
> bit careless and copied / replaced too much code ...

    Actually, the need to check if each char >= 0x80 would complicate
the use of bulk writes, so I can understand the removal.  I was
more indicating that this was an important feature of the previous
code.

    [...]  Lots of stuff deleted.

>> Really annoy bits: ;)
>>   You seem to have been a bit careless with the Copyright lines.
>>   Most of the new classes say 2001, in a few cases you deleted
>>   existing years in a Copyright.
> 
> Really I am unlucky and careless !! I picked up a file in Batik SVN 
> repository at random and I must have accidentally stumbled on classes 
> which have the old header (I should have checked after, I know...). It 
> turns out that some of the classes still have the old header. Not a 
> reason for me to add new ones ;)

     Actually the Copyright year(s) should only be updated when the
file is actually modified.  So there will be lots of 'old' years
in the Batik repository, since there are sections of code that aren't
updated all the time.  In short you need to add 2005 to the Copyright
years list if it already has it, for new files just have 2005.

>> Finally:
>>    Can I add the 'test' classes to Batik?  They have the Apache License
>>    so I assume so but I just wanted to make sure.  I will probably
>>    add the WMF viewer to the 'contrib' directory and try and figure
>>    out how to merge the test transcoder into the Batik test framework.
> 
> Yes, I put the licence header in those files so you could put it in some 
> Batik directory if you thought they could be useful.

   Ok, thanks!

> If you want, I can modify the files according to your remarks and file 
> the new version of the patch attached to the RFE.

   This would be ideal!

---------------------------------------------------------------------
To unsubscribe, e-mail: batik-dev-unsubscribe@xmlgraphics.apache.org
For additional commands, e-mail: batik-dev-help@xmlgraphics.apache.org


Re: Enhancements in WMF transcoder

Posted by Hervé Girod <he...@club-internet.fr>.
Hello Thomas,

Thank for your comments. here are my answers :

Thomas DeWeese wrote:

> XmlWriter changes right now I'm not going to accept due to some issues:
>   1) Your updates removed the use of bulk writes which was
>      specifically added due to performance issues.

Sorry, maybe I first modify this file before this change, and I was a 
bit careless and copied / replaced too much code ...

>   2) The change to encode all chars >= 0x80 as hex is not friendly
>      for foreign language users (they might view the document in an
>      editor that really displays those things).  This is one of the
>      intents of encoding to handle.  At most this might be made a option
>      for when writing the document.

You are right.

> SVGGraphics2D
>
> Line 223 in patch:
>   What's with the null paint stuff?  It looks like it isn't used.

I wrote this part some time (I think a long time) ago to convert an old 
proprietary graphic format were there was a "non visible" color, but I 
didn't choose the right way to do this, and later the code remained 
there, harmless but not useful. So : this code is not really used and 
can be removed. 

> Lines 272 & 292 in patch:
>   I'm not sure that the 'solution' of setting stroke to the
>   'textColor' is a good one - most text doesn't want to be stroked
>   at all.  Can you describe exactly what the problem is?
>   Should we be setting stroke to 'none'?

I'm not able to see any more what was my problem at the moment (I'm 
pretty sure I encountered one that dealed with definitions of text 
characteristics higher in the SVG tree than the 'text' tag, but I can't 
reproduce the problem for the moment, and beside it could have been my 
fault ...). I think it better to get back to the original code for that 
part. I will file a bug if I finally find a SVG file to reproduce the 
original problem (an if there was really a problem at all).

> Line 289:
>   In what case are we missing setting the font-size correctly?
>   Is this for cases where you are using 'getRoot' repeatedly?
>   If so then much more needs to be done I think...

Same as the above point.

> Line 305:
>   You deleted the restoration of the regular transform after a
>   supplemental transform was needed to draw text (needed for very
>   small fonts).

I think that's for the same reason as for your first remark : careless 
copy / replace of code.

> Line 329:
>   While I appreciate the very good attempt at handling of attributed
>   Character iterators, I am concerned that the code only considers
>   the attributes on the first char.  

I needed this to handle easily decorated text in WMF file. I began 
simple, and I thought that I would handle the attributes on other chars 
as well, but later I forgot about it...

> These attributes can change
>   on a per char basis - requiring the creation of tspans.  Perhaps
>   check code can be added to see if the attributes are constant
>   (see 'ACI.getRunLimit(Set attributes)' and use the improved code
>   in that case and use the outline path it in the more complex cases?

That's a good idea.

> Line 430:
>   I'm not thrilled with the mapping of Justifications.  Once again
>   I would lean towards mapping zero to 'start', one to 'end'
>   .5 to middle, any other value can either be done with 'dx' or
>   simply stroke it.  This should handle the most common cases nicely
>   and not cause regressions for the other cases.

Yes, using  'dx' could be better. putting CSS_MIDDLE_VALUE for values 
between 0.3 and 0.7 was not a so bright idea, after looking at it again... 

> Line 3936:
>   .nbattrs (Net beans something file) in wmf/tosvg needed?

No sorry, it is just a Netbeans file, don't now why it landed there...

> Really annoy bits: ;)
>   You seem to have been a bit careless with the Copyright lines.
>   Most of the new classes say 2001, in a few cases you deleted
>   existing years in a Copyright.

Really I am unlucky and careless !! I picked up a file in Batik SVN 
repository at random and I must have accidentally stumbled on classes 
which have the old header (I should have checked after, I know...). It 
turns out that some of the classes still have the old header. Not a 
reason for me to add new ones ;)

> Finally:
>    Can I add the 'test' classes to Batik?  They have the Apache License
>    so I assume so but I just wanted to make sure.  I will probably
>    add the WMF viewer to the 'contrib' directory and try and figure
>    out how to merge the test transcoder into the Batik test framework.

Yes, I put the licence header in those files so you could put it in some 
Batik directory if you thought they could be useful.

If you want, I can modify the files according to your remarks and file 
the new version of the patch attached to the RFE.



---------------------------------------------------------------------
To unsubscribe, e-mail: batik-dev-unsubscribe@xmlgraphics.apache.org
For additional commands, e-mail: batik-dev-help@xmlgraphics.apache.org


Re: Enhancements in WMF transcoder

Posted by Thomas DeWeese <Th...@Kodak.com>.
Hi Hervé,

    So I've looked the patch over quite a bit and I have some
questions/comments.  In general stuff looks good, I'm sure
people dealing with WMF files will appreciate the significant
improvements there, unfortunately I know nothing about WMF so
I don't have much to comment on that work.

XmlWriter changes right now I'm not going to accept due to some issues:
   1) Your updates removed the use of bulk writes which was
      specifically added due to performance issues.
   2) The change to encode all chars >= 0x80 as hex is not friendly
      for foreign language users (they might view the document in an
      editor that really displays those things).  This is one of the
      intents of encoding to handle.  At most this might be made a option
      for when writing the document.

SVGGraphics2D

Line 223 in patch:
   What's with the null paint stuff?  It looks like it isn't used.

Lines 272 & 292 in patch:
   I'm not sure that the 'solution' of setting stroke to the
   'textColor' is a good one - most text doesn't want to be stroked
   at all.  Can you describe exactly what the problem is?
   Should we be setting stroke to 'none'?

Line 289:
   In what case are we missing setting the font-size correctly?
   Is this for cases where you are using 'getRoot' repeatedly?
   If so then much more needs to be done I think...

Line 305:
   You deleted the restoration of the regular transform after a
   supplemental transform was needed to draw text (needed for very
   small fonts).

Line 329:
   While I appreciate the very good attempt at handling of attributed
   Character iterators, I am concerned that the code only considers
   the attributes on the first char.  These attributes can change
   on a per char basis - requiring the creation of tspans.  Perhaps
   check code can be added to see if the attributes are constant
   (see 'ACI.getRunLimit(Set attributes)' and use the improved code
   in that case and use the outline path it in the more complex cases?
   Otherwise I'm afraid people will see
   regressions (text will be text but improperly styled).
   This is a good start towards generating the tspans (essentially the
   same code needs to be done for each tspan).

Line 430:
   I'm not thrilled with the mapping of Justifications.  Once again
   I would lean towards mapping zero to 'start', one to 'end'
   .5 to middle, any other value can either be done with 'dx' or
   simply stroke it.  This should handle the most common cases nicely
   and not cause regressions for the other cases.

Line 3936:
   .nbattrs (Net beans something file) in wmf/tosvg needed?

Really annoy bits: ;)
   You seem to have been a bit careless with the Copyright lines.
   Most of the new classes say 2001, in a few cases you deleted
   existing years in a Copyright.

Finally:
    Can I add the 'test' classes to Batik?  They have the Apache License
    so I assume so but I just wanted to make sure.  I will probably
    add the WMF viewer to the 'contrib' directory and try and figure
    out how to merge the test transcoder into the Batik test framework.

Thanks again this looks like a big improvement in the WMF stuff,
as well as some potentially very useful improvements to the
SVGGraphics2D.


----

Thomas DeWeese wrote:
> Hi Hervé,



> 
> Hervé Girod wrote:
> 
>>>> I have enhanced / hacked the actual transcoder code (as of version 
>>>> 1.6) and added / corrected the following functionalities :
> 
> [...]
> 
>> OK,  it is done (much more mork than I expected, since I tried to 
>> "beautify" my code a little). Also I added image / texture handling.
> 
> 
>    Thanks I'll look at integrating this soon.
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: batik-dev-unsubscribe@xmlgraphics.apache.org
> For additional commands, e-mail: batik-dev-help@xmlgraphics.apache.org
> 


---------------------------------------------------------------------
To unsubscribe, e-mail: batik-dev-unsubscribe@xmlgraphics.apache.org
For additional commands, e-mail: batik-dev-help@xmlgraphics.apache.org


Re: Enhancements in WMF transcoder

Posted by Thomas DeWeese <Th...@Kodak.com>.
Hi Hervé,

Hervé Girod wrote:

>>> I have enhanced / hacked the actual transcoder code (as of version 
>>> 1.6) and added / corrected the following functionalities :
[...]
> OK,  it is done (much more mork than I expected, since I tried to 
> "beautify" my code a little). Also I added image / texture handling.

    Thanks I'll look at integrating this soon.

---------------------------------------------------------------------
To unsubscribe, e-mail: batik-dev-unsubscribe@xmlgraphics.apache.org
For additional commands, e-mail: batik-dev-help@xmlgraphics.apache.org