You are viewing a plain text version of this content. The canonical link for it is here.
Posted to fop-commits@xmlgraphics.apache.org by je...@apache.org on 2009/02/09 23:09:40 UTC
svn commit: r742766 - in /xmlgraphics/fop/trunk/src/java/org/apache/fop/pdf:
PDFT1Stream.java PDFTTFStream.java
Author: jeremias
Date: Mon Feb 9 22:09:40 2009
New Revision: 742766
URL: http://svn.apache.org/viewvc?rev=742766&view=rev
Log:
There was a filter list constant for fonts, but our font streams didn't use it.
Modified:
xmlgraphics/fop/trunk/src/java/org/apache/fop/pdf/PDFT1Stream.java
xmlgraphics/fop/trunk/src/java/org/apache/fop/pdf/PDFTTFStream.java
Modified: xmlgraphics/fop/trunk/src/java/org/apache/fop/pdf/PDFT1Stream.java
URL: http://svn.apache.org/viewvc/xmlgraphics/fop/trunk/src/java/org/apache/fop/pdf/PDFT1Stream.java?rev=742766&r1=742765&r2=742766&view=diff
==============================================================================
--- xmlgraphics/fop/trunk/src/java/org/apache/fop/pdf/PDFT1Stream.java (original)
+++ xmlgraphics/fop/trunk/src/java/org/apache/fop/pdf/PDFT1Stream.java Mon Feb 9 22:09:40 2009
@@ -87,4 +87,14 @@
this.pfb = pfb;
}
+ /** {@inheritDoc} */
+ protected void setupFilterList() {
+ if (!getFilterList().isInitialized()) {
+ getFilterList().addDefaultFilters(
+ getDocumentSafely().getFilterMap(),
+ PDFFilterList.FONT_FILTER);
+ }
+ super.setupFilterList();
+ }
+
}
Modified: xmlgraphics/fop/trunk/src/java/org/apache/fop/pdf/PDFTTFStream.java
URL: http://svn.apache.org/viewvc/xmlgraphics/fop/trunk/src/java/org/apache/fop/pdf/PDFTTFStream.java?rev=742766&r1=742765&r2=742766&view=diff
==============================================================================
--- xmlgraphics/fop/trunk/src/java/org/apache/fop/pdf/PDFTTFStream.java (original)
+++ xmlgraphics/fop/trunk/src/java/org/apache/fop/pdf/PDFTTFStream.java Mon Feb 9 22:09:40 2009
@@ -70,4 +70,14 @@
getBufferOutputStream().write(data, 0, size);
}
+ /** {@inheritDoc} */
+ protected void setupFilterList() {
+ if (!getFilterList().isInitialized()) {
+ getFilterList().addDefaultFilters(
+ getDocumentSafely().getFilterMap(),
+ PDFFilterList.FONT_FILTER);
+ }
+ super.setupFilterList();
+ }
+
}
---------------------------------------------------------------------
To unsubscribe, e-mail: fop-commits-unsubscribe@xmlgraphics.apache.org
For additional commands, e-mail: fop-commits-help@xmlgraphics.apache.org
Re: svn commit: r742766 - in /xmlgraphics/fop/trunk/src/java/org/apache/fop/pdf:
PDFT1Stream.java PDFTTFStream.java
Posted by Vincent Hennebert <vh...@gmail.com>.
Hi,
Jeremias Maerki wrote:
> On 10.02.2009 20:26:19 Andreas Delmelle wrote:
>> On 10 Feb 2009, at 12:33, Vincent Hennebert wrote:
<snip/>
>>
>> That said, I also can't immediately say how many font-stream types
>> there are or should be, but if it's only two classes and it is
>> unlikely that others will become necessary, one could start to wonder
>> whether it's worth the effort...
>
> There are two font-stream types at the moment: Type 1 and TTF.
The setupFilterList method is defined or re-defined 5 times in the
hierarchy, and the 5 versions are almost identical. Besides the font
streams, something’s definitely wrong there.
> AbstractPDFStream (or a common AbstractFontStream) without the use of
> PDFStream inside its code. At the moment, the TTF data is buffered
> twice: once as a byte[] in PDFFactory and once inside the PDFStream's
> internal buffer. Doesn't make too much sense. I'll take a closer look
> when I have time (and motivation). I have other priorities right now. If
> that's a problem I can also revert revision 742766.
+1
Thanks,
Vincent
Re: svn commit: r742766 - in /xmlgraphics/fop/trunk/src/java/org/apache/fop/pdf: PDFT1Stream.java PDFTTFStream.java
Posted by Jeremias Maerki <de...@jeremias-maerki.ch>.
On 10.02.2009 20:26:19 Andreas Delmelle wrote:
> On 10 Feb 2009, at 12:33, Vincent Hennebert wrote:
>
> >> Author: jeremias
> >> Date: Mon Feb 9 22:09:40 2009
> >> New Revision: 742766
> >> <snip />
>
> > And yet another copy-paste. Was that really impossible to factorize
> > this
> > piece of code in a common super class?
>
> Heh... Well, I guess we could always /try/ to factorize out every
> duplicate conditional. :-]
>
> Definitely not impossible, but since PDFTTFStream extends PDFStream
> and PDFT1Stream extends AbstractPDFStream, this would probably lead to
> other code being either unnecessarily duplicated or inherited, unless
> it is carefully thought through.
>
> The key lies in that very simple one-liner that follows the 'if':
>
> super.setupFilterList();
>
> Currently, there is only one common implementation in
> AbstractPDFStream. If someone were to implement it in PDFStream
> differently, that piece of code in question, while identical /code/
> for both classes, could come to mean something different...
>
> True, it doesn't, so one could add something like an
> AbstractPDFFontStream to hold this one small method setupFilterList().
>
> Since PDFTTFStream uses some members/methods in PDFStream, the choice
> then becomes:
> * push those members/methods up (so some other classes, which do not
> really need them, like PDFT1Stream, inherit them as well, possibly
> with the consequence of necessitating overrides to correct behavioral
> changes introduced by the abstract parent, and make the concrete
> subclass act like its abstract grandparent --copy-paste?), or
> * copy what is needed from PDFStream, and paste it into PDFTTFStream,
> then sever the link and have PDFTTFStream extend AbstractPDFFontStream
> * implement PDFTTFStream as a proxy to PDFStream. See attached patch.
> Untested; I have not yet verified whether there other pieces of the
> code that rely on PDFTTFStream extending PDFStream.
>
> That said, I also can't immediately say how many font-stream types
> there are or should be, but if it's only two classes and it is
> unlikely that others will become necessary, one could start to wonder
> whether it's worth the effort...
There are two font-stream types at the moment: Type 1 and TTF.
I think it would be possible to derive PDFTTFStream from
AbstractPDFStream (or a common AbstractFontStream) without the use of
PDFStream inside its code. At the moment, the TTF data is buffered
twice: once as a byte[] in PDFFactory and once inside the PDFStream's
internal buffer. Doesn't make too much sense. I'll take a closer look
when I have time (and motivation). I have other priorities right now. If
that's a problem I can also revert revision 742766.
Jeremias Maerki
Re: svn commit: r742766 - in /xmlgraphics/fop/trunk/src/java/org/apache/fop/pdf: PDFT1Stream.java PDFTTFStream.java
Posted by Andreas Delmelle <an...@telenet.be>.
On 10 Feb 2009, at 12:33, Vincent Hennebert wrote:
>> Author: jeremias
>> Date: Mon Feb 9 22:09:40 2009
>> New Revision: 742766
>> <snip />
> And yet another copy-paste. Was that really impossible to factorize
> this
> piece of code in a common super class?
Heh... Well, I guess we could always /try/ to factorize out every
duplicate conditional. :-]
Definitely not impossible, but since PDFTTFStream extends PDFStream
and PDFT1Stream extends AbstractPDFStream, this would probably lead to
other code being either unnecessarily duplicated or inherited, unless
it is carefully thought through.
The key lies in that very simple one-liner that follows the 'if':
super.setupFilterList();
Currently, there is only one common implementation in
AbstractPDFStream. If someone were to implement it in PDFStream
differently, that piece of code in question, while identical /code/
for both classes, could come to mean something different...
True, it doesn't, so one could add something like an
AbstractPDFFontStream to hold this one small method setupFilterList().
Since PDFTTFStream uses some members/methods in PDFStream, the choice
then becomes:
* push those members/methods up (so some other classes, which do not
really need them, like PDFT1Stream, inherit them as well, possibly
with the consequence of necessitating overrides to correct behavioral
changes introduced by the abstract parent, and make the concrete
subclass act like its abstract grandparent --copy-paste?), or
* copy what is needed from PDFStream, and paste it into PDFTTFStream,
then sever the link and have PDFTTFStream extend AbstractPDFFontStream
* implement PDFTTFStream as a proxy to PDFStream. See attached patch.
Untested; I have not yet verified whether there other pieces of the
code that rely on PDFTTFStream extending PDFStream.
That said, I also can't immediately say how many font-stream types
there are or should be, but if it's only two classes and it is
unlikely that others will become necessary, one could start to wonder
whether it's worth the effort...
Re: svn commit: r742766 - in /xmlgraphics/fop/trunk/src/java/org/apache/fop/pdf:
PDFT1Stream.java PDFTTFStream.java
Posted by Vincent Hennebert <vh...@gmail.com>.
> Author: jeremias
> Date: Mon Feb 9 22:09:40 2009
> New Revision: 742766
>
> URL: http://svn.apache.org/viewvc?rev=742766&view=rev
> Log:
> There was a filter list constant for fonts, but our font streams didn't use it.
>
> Modified:
> xmlgraphics/fop/trunk/src/java/org/apache/fop/pdf/PDFT1Stream.java
> xmlgraphics/fop/trunk/src/java/org/apache/fop/pdf/PDFTTFStream.java
>
> Modified: xmlgraphics/fop/trunk/src/java/org/apache/fop/pdf/PDFT1Stream.java
> URL: http://svn.apache.org/viewvc/xmlgraphics/fop/trunk/src/java/org/apache/fop/pdf/PDFT1Stream.java?rev=742766&r1=742765&r2=742766&view=diff
> ==============================================================================
> --- xmlgraphics/fop/trunk/src/java/org/apache/fop/pdf/PDFT1Stream.java (original)
> +++ xmlgraphics/fop/trunk/src/java/org/apache/fop/pdf/PDFT1Stream.java Mon Feb 9 22:09:40 2009
> @@ -87,4 +87,14 @@
> this.pfb = pfb;
> }
>
> + /** {@inheritDoc} */
> + protected void setupFilterList() {
> + if (!getFilterList().isInitialized()) {
> + getFilterList().addDefaultFilters(
> + getDocumentSafely().getFilterMap(),
> + PDFFilterList.FONT_FILTER);
> + }
> + super.setupFilterList();
> + }
> +
> }
>
> Modified: xmlgraphics/fop/trunk/src/java/org/apache/fop/pdf/PDFTTFStream.java
> URL: http://svn.apache.org/viewvc/xmlgraphics/fop/trunk/src/java/org/apache/fop/pdf/PDFTTFStream.java?rev=742766&r1=742765&r2=742766&view=diff
> ==============================================================================
> --- xmlgraphics/fop/trunk/src/java/org/apache/fop/pdf/PDFTTFStream.java (original)
> +++ xmlgraphics/fop/trunk/src/java/org/apache/fop/pdf/PDFTTFStream.java Mon Feb 9 22:09:40 2009
> @@ -70,4 +70,14 @@
> getBufferOutputStream().write(data, 0, size);
> }
>
> + /** {@inheritDoc} */
> + protected void setupFilterList() {
> + if (!getFilterList().isInitialized()) {
> + getFilterList().addDefaultFilters(
> + getDocumentSafely().getFilterMap(),
> + PDFFilterList.FONT_FILTER);
> + }
> + super.setupFilterList();
> + }
> +
> }
And yet another copy-paste. Was that really impossible to factorize this
piece of code in a common super class?
Vincent