You are viewing a plain text version of this content. The canonical link for it is here.
Posted to fop-dev@xmlgraphics.apache.org by Matthew L Daniel <md...@scdi.com> on 2005/09/20 00:28:56 UTC
rtf ignoring TableBody et al
I am using trunk code and with an input FO that produces excellent PDF
output, I am seeing the following messages about lots of ignored
instances.
After checking RTFHandler.java, it seems they are omitted from the very
large instance-switch in "invokeDeferredEvent".
My question is if this is "in progress" code or if RTFHandler has been
broken like this for a while?
I tried to be clever and hack support for them back in, but it went very
poorly.
Thanks for your time,
-- /v\atthew
> Ignored deferred event for org.apache.fop.fo.flow.TableBody@2c766a24
> Ignored deferred event for org.apache.fop.fo.flow.ExternalGraphic@56da6bf4
> Ignored deferred event for org.apache.fop.fo.flow.ExternalGraphic@56da6bf4
> Ignored deferred event for org.apache.fop.fo.flow.TableBody@2c766a24
> Ignored deferred event for org.apache.fop.fo.flow.InstreamForeignObject@1de58cb8
> Ignored deferred event for org.apache.fop.fo.extensions.svg.SVGElement@4979935d
> Ignored deferred event for org.apache.fop.fo.extensions.svg.SVGElement@4979935d
> Ignored deferred event for org.apache.fop.fo.flow.InstreamForeignObject@1de58cb8
> Ignored deferred event for org.apache.fop.fo.flow.TableBody@4cb9e45a
> Ignored deferred event for org.apache.fop.fo.flow.TableBody@403ef810
> Ignored deferred event for org.apache.fop.fo.flow.TableBody@403ef810
> Ignored deferred event for org.apache.fop.fo.flow.TableHeader@66100363
> Ignored deferred event for org.apache.fop.fo.flow.TableHeader@66100363
> ... this continues for quite a while ...
Re: rtf ignoring TableBody et al
Posted by Jeremias Maerki <de...@jeremias-maerki.ch>.
On 20.09.2005 17:50:03 Matthew L Daniel wrote:
> > The warning is an indicator that there are still a few things in RTF
> > support that need fixing.
>
> So is there someone who owns this class?
No. There are no class owners in Apache projects.
> How long has it been
> out-of-sync - that is, is it in the same condition as TXTRenderer?
The TXTRenderer has not been implemented after the redesign. The RTF
support was added by integrating JFOR. The RTF support works reasonably
well for many use cases but most probably not all. It's still
alpha-grade code like the rest of the redesigned FOP.
> Also, while this is probably not the correct forum for this question,
> what has been the reference tool for checking the RTF output?
The RTF support is mainly for MS Word. I use the MS Word 2003 Viewer.
> OpenOffice.org 2.0 beta won't even correctly render the SimpleTable from
> jfor. Since Word Viewer 2003 does, I'm inclined to think it's an OOo
> bug more than a jfor bug.
OpenOffice is horrible when it comes to RTF. We'd better create a
develop FOEventHandler for the new OpenDocument format if someone wants
to open FOP-generated documents in OpenOffice.
> However, I hope this he-said-she-said
> illustrates my correctness concern for the RTF output.
I don't understand what you mean. Probably a foreign language parse
error on my side. :-)
Jeremias Maerki
AW: rtf ignoring TableBody et al
Posted by Peter Herweg <ph...@web.de>.
Jeremias Maerki wrote:
> I added the warning because I found a case where a block-container
> wasn't handled as it should have been:
> http://svn.apache.org/viewcvs?rev=278720&view=rev
Good idea, in my opinion.
Regards,
Peter Herweg
AW: rtf ignoring TableBody et al
Posted by Peter Herweg <ph...@web.de>.
I am checking the rtf results with Word 2000 SP3.
Regards,
Peter Herweg
-----Ursprungliche Nachricht-----
Von: fop-dev-return-28038-pherweg=web.de@xmlgraphics.apache.org
[mailto:fop-dev-return-28038-pherweg=web.de@xmlgraphics.apache.org]Im
Auftrag von Matthew L Daniel
Gesendet: Dienstag, 20. September 2005 17:50
An: fop-dev@xmlgraphics.apache.org
Betreff: Re: rtf ignoring TableBody et al
> The warning is an indicator that there are still a few things in RTF
> support that need fixing.
So is there someone who owns this class? How long has it been
out-of-sync - that is, is it in the same condition as TXTRenderer?
Also, while this is probably not the correct forum for this question,
what has been the reference tool for checking the RTF output?
OpenOffice.org 2.0 beta won't even correctly render the SimpleTable from
jfor. Since Word Viewer 2003 does, I'm inclined to think it's an OOo
bug more than a jfor bug. However, I hope this he-said-she-said
illustrates my correctness concern for the RTF output.
-- /v\atthew
Re: rtf ignoring TableBody et al
Posted by Matthew L Daniel <md...@scdi.com>.
> The warning is an indicator that there are still a few things in RTF
> support that need fixing.
So is there someone who owns this class? How long has it been
out-of-sync - that is, is it in the same condition as TXTRenderer?
Also, while this is probably not the correct forum for this question,
what has been the reference tool for checking the RTF output?
OpenOffice.org 2.0 beta won't even correctly render the SimpleTable from
jfor. Since Word Viewer 2003 does, I'm inclined to think it's an OOo
bug more than a jfor bug. However, I hope this he-said-she-said
illustrates my correctness concern for the RTF output.
-- /v\atthew
Re: rtf ignoring TableBody et al
Posted by Jeremias Maerki <de...@jeremias-maerki.ch>.
I added the warning because I found a case where a block-container
wasn't handled as it should have been:
http://svn.apache.org/viewcvs?rev=278720&view=rev
The warning is an indicator that there are still a few things in RTF
support that need fixing.
On 20.09.2005 00:28:56 Matthew L Daniel wrote:
> I am using trunk code and with an input FO that produces excellent PDF
> output, I am seeing the following messages about lots of ignored
> instances.
>
> After checking RTFHandler.java, it seems they are omitted from the very
> large instance-switch in "invokeDeferredEvent".
>
> My question is if this is "in progress" code or if RTFHandler has been
> broken like this for a while?
>
> I tried to be clever and hack support for them back in, but it went very
> poorly.
>
> Thanks for your time,
> -- /v\atthew
>
> > Ignored deferred event for org.apache.fop.fo.flow.TableBody@2c766a24
> > Ignored deferred event for org.apache.fop.fo.flow.ExternalGraphic@56da6bf4
> > Ignored deferred event for org.apache.fop.fo.flow.ExternalGraphic@56da6bf4
> > Ignored deferred event for org.apache.fop.fo.flow.TableBody@2c766a24
> > Ignored deferred event for org.apache.fop.fo.flow.InstreamForeignObject@1de58cb8
> > Ignored deferred event for org.apache.fop.fo.extensions.svg.SVGElement@4979935d
> > Ignored deferred event for org.apache.fop.fo.extensions.svg.SVGElement@4979935d
> > Ignored deferred event for org.apache.fop.fo.flow.InstreamForeignObject@1de58cb8
> > Ignored deferred event for org.apache.fop.fo.flow.TableBody@4cb9e45a
> > Ignored deferred event for org.apache.fop.fo.flow.TableBody@403ef810
> > Ignored deferred event for org.apache.fop.fo.flow.TableBody@403ef810
> > Ignored deferred event for org.apache.fop.fo.flow.TableHeader@66100363
> > Ignored deferred event for org.apache.fop.fo.flow.TableHeader@66100363
> > ... this continues for quite a while ...
Jeremias Maerki
AW: rtf ignoring TableBody et al
Posted by Peter Herweg <ph...@web.de>.
Yes, there are few nodes types missing, because i just added the node types,
which are implemented in rtf module.
But you are right: TableBody is implemented, but is missing in the
if-structure. I'll add it.
There is also support for ExternalGraphics, but i guess this won't work so i
did not add it to if-structure. This will be my next task in rtf module.
Regards,
Peter Herweg
-----Ursprungliche Nachricht-----
Von: fop-dev-return-28019-pherweg=web.de@xmlgraphics.apache.org
[mailto:fop-dev-return-28019-pherweg=web.de@xmlgraphics.apache.org]Im
Auftrag von Andreas L Delmelle
Gesendet: Dienstag, 20. September 2005 00:58
An: fop-dev@xmlgraphics.apache.org
Betreff: Re: rtf ignoring TableBody et al
On Sep 20, 2005, at 00:28, Matthew L Daniel wrote:
> I am using trunk code and with an input FO that produces excellent PDF
> output, I am seeing the following messages about lots of ignored
> instances.
>
> After checking RTFHandler.java, it seems they are omitted from the very
> large instance-switch in "invokeDeferredEvent".
>
> My question is if this is "in progress" code or if RTFHandler has been
> broken like this for a while?
Seems like it's broken. At least, there's a few node-types missing,
amongst which precisely TableBody, TableHeader, TableFooter,
InstreamForeignObject, SVGElement, ExternalGraphic...
I'm going to leave this for now, since I don't know too much about the
RTF Rendering code --maybe those node-types were left out for a good
reason, I'm not sure.
At first glance, the whole structure:
if( fo instanceof PageSequence ) {
//do something
} else if( fo instanceof Flow ) {
//do something
} ...
would probably better be replaced by one
switch( fo.getNameId() ) {
case Constants.FO_PAGE_SEQUENCE:
//do something
break;
case Constants.FO_FLOW:
//do something
break;
...
}
The least this would do is avoid a number of unnecessary calls to
instanceof.
Note to Peter Herweg: I think I see now what you meant by the many
'if-then-else-if's. I hope you can put the above comment to good use...
Cheers,
Andreas
Re: rtf ignoring TableBody et al
Posted by Andreas L Delmelle <a_...@pandora.be>.
On Sep 20, 2005, at 17:40, Matthew L Daniel wrote:
Hi Matthew,
>> The least this would do is avoid a number of unnecessary calls to
>> instanceof.
>
> You're on the right track, and maybe that "switch" impl would be an
> 80/20 win over the cost of the Right Way. But that huge if-instance
> case
> is exactly what the Visitor pattern was created to solve.
Yep! My remark was completely focused on that isolated part in one of
the +600 source files. It just seemed like the more elegant way to deal
with it --since the FONodes all have that getNameId() method, it makes
more sense to call that once and compare the resulting int value to the
constants, rather than checking the class for one node possibly so many
times.
(Plus: using 'switch' offers the benefit of having fall-through cases,
where you would need 'if-instanceof||instanceof||instanceof...')
As a personal note: I'd even like to replace all string-comparisons of
the form 'fo.getName().equals("fo:something")' in the FOTree with
'fo.getNameId() == FO_SOMETHING'. Not sure if that would save us much,
but every little bit helps I guess. Even if both strings are
internalized, 'int == int' outperforms 'String.equals(String)' by a
significant factor (and that's still leaving small things like
"fo:instream-foreign-object" [=26 Bytes] vs. FO_INSTREAM_FOREIGN_OBJECT
[=4 Bytes] out of the picture)
'instanceof' itself can IMO only be put to good use in case of a
complex class-hierarchy, where you can check for supertypes that each
have lots-and-lots of subtypes, so 'x instanceof supertype' covers ten
or more subtypes in one go.
Our FOTree isn't exactly suited for that kind of thing. Practically all
the FOs are direct subclasses of FObj, and those that aren't, have at
most one intermediate class in between...
(I've taken the liberty of adding one especially for the table-related
FOs, since they share some functionality --still have to take a closer
look at which members can be moved upward to TableFObj. For now, it's
restricted to a few methods for dealing with the column-numbering and
the border-precedences, but I'm almost certain that more can be made to
follow.)
> I haven't spent enough time with the Fop codebase to speak to the
> gains of such a
> thing, but I'd bet RTFHandler isn't the only one that does this.
Indeed not. FYI: FOP once did use the Visitor pattern for the
layout-engine (over a year ago), but it was decided upon back then to
remove it completely, mainly for reasons of keeping the code
comprehensible for potential future committers IIRC.
> A braindead `grep instanceof | wc -l` search shows 122 classes out of
> the
> 640 classes use instanceof, which in my world makes them at least worth
> examination.
Depends a bit on *what* exactly they're checking and how many times
they actually do that. If it's simple checks for FONode subtypes that
are executed lots of times, these are indeed most likely suboptimal...
> Another "implied" win to the Visitor interface is if a new FONode type
> gets added, it forces the change to ripple (unless the impl is using
> the
> DefaultVisitor) to all those who would need to handle the new
> functionality.
... which indeed becomes of very much use once you start talking about
FO Extension-elements.
Thanks for the valuable feedback --and for being one of our
guinea-pigs, of course ;-)
Cheers,
Andreas
Re: rtf ignoring TableBody et al
Posted by Matthew L Daniel <md...@scdi.com>.
> The least this would do is avoid a number of unnecessary calls to
> instanceof.
You're on the right track, and maybe that "switch" impl would be an
80/20 win over the cost of the Right Way. But that huge if-instance case
is exactly what the Visitor pattern was created to solve. I haven't
spent enough time with the Fop codebase to speak to the gains of such a
thing, but I'd bet RTFHandler isn't the only one that does this. A
braindead `grep instanceof | wc -l` search shows 122 classes out of the
640 classes use instanceof, which in my world makes them at least worth
examination.
Another "implied" win to the Visitor interface is if a new FONode type
gets added, it forces the change to ripple (unless the impl is using the
DefaultVisitor) to all those who would need to handle the new
functionality.
-- /v\atthew
Re: rtf ignoring TableBody et al
Posted by Andreas L Delmelle <a_...@pandora.be>.
On Sep 20, 2005, at 00:28, Matthew L Daniel wrote:
> I am using trunk code and with an input FO that produces excellent PDF
> output, I am seeing the following messages about lots of ignored
> instances.
>
> After checking RTFHandler.java, it seems they are omitted from the very
> large instance-switch in "invokeDeferredEvent".
>
> My question is if this is "in progress" code or if RTFHandler has been
> broken like this for a while?
Seems like it's broken. At least, there's a few node-types missing,
amongst which precisely TableBody, TableHeader, TableFooter,
InstreamForeignObject, SVGElement, ExternalGraphic...
I'm going to leave this for now, since I don't know too much about the
RTF Rendering code --maybe those node-types were left out for a good
reason, I'm not sure.
At first glance, the whole structure:
if( fo instanceof PageSequence ) {
//do something
} else if( fo instanceof Flow ) {
//do something
} ...
would probably better be replaced by one
switch( fo.getNameId() ) {
case Constants.FO_PAGE_SEQUENCE:
//do something
break;
case Constants.FO_FLOW:
//do something
break;
...
}
The least this would do is avoid a number of unnecessary calls to
instanceof.
Note to Peter Herweg: I think I see now what you meant by the many
'if-then-else-if's. I hope you can put the above comment to good use...
Cheers,
Andreas