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 Venkat Reddy <va...@googlemail.com> on 2009/10/15 17:45:59 UTC
Making AFPPaintingState singleton
Hi,
AFPPaintingState is being used in three different places altogether in
FopTrunk source. The default constructor is being used in the following
three classes...
1. AFPDocumentHandler.java
2. AFPRenderer.java
3. AFPPageOverlayElement.java
There is a variable 'resolution' is being initialized for each instance,
this resolution parameter can be set using the 'fop.xconf' for a
particular render...
Ex:- AFPRenderer configuration below
<renderer mime="application/x-afp">
<!--
The bit depth and type of images produced
(this is the default setting)
-->
<images mode="b+w" bits-per-pixel="8"/>
<renderer-resolution>1400</renderer-resolution>
The above <renderer-resolution> is being hardcoded as '240dpi' in
AFPRendererConfigurator.java, which initiates the renderer resolution
based on the configuration set in 'fop.xconf'. In order to resolve this
problem, I will be changing the AFPPaintingState as singleton, so that
all the above classes will get the instance using 'getInstance()' method
instead of default constructor. This will resolve the
<renderer-resolution> problem as well, by a simple change in
AFPRendererConfigurator (instead of hardcoded value 240, assigning the
value from the configuration object).
Please review the above changes and tell me, if I am doing anything
wrong here?
Thanks,
Venkat.
Re: Making AFPPaintingState singleton
Posted by Adrian Cumiskey <de...@cumiskey.com>.
Hi Venkat,
I advise you to just pass the bare values in AFPPageOverlayElement from
attlist.getValue(ATT_X) and attlist.getValue(ATT_Y) through to the
AFPPageOverlay and remove the use of AFPPaintingState and
AFPUnitConverter in there - this was never the correct place to do
this. You could look at calling upon AFPUnitConverter to carry out the
correct plotting calculation on your AFPPaintingState instance member
variable in DataStream.createIncludePageOverlay(String name, int x, int
y). This would mean regardless of whether FOP is using the
AFPDocumentHandler or the AFPRenderer implementation, the calculation
will still be done correctly and at the right time. Hope this helps you
with the fix.
Adrian.
Venkat Reddy wrote:
> Hi Adrian,
>
> Thanks for your reply.
>
> Yes, You are absolutely right, I am trying to handle these conversions
> at the renderer level... I will soon come out with a patch to the bug
> you opened.
>
> Thanks once again for the source points,
> Venkat.
>
> Adrian Cumiskey wrote:
>> Hi Venkat,
>>
>> This approach is not a good idea. It is possible that a runtime
>> environment is configured to have multiple instances of Fop being
>> instantiated by FopFactory with different rendering configurations
>> (e.g. renderer resolution values).
>>
>> Its disappointing, but I've just noticed a bug in
>> AFPPageOverlayElement, an AFPPaintingState shouldn't just be
>> instantiated in there like that. The processNode implementation will
>> not accurately calculate and plot the page overlay position if the
>> document resolution is different from the detault value of 240dpi.
>> This calculation should be carried out much later at rendering time,
>> *not* in here at configuration time - its very hacky and you'll need
>> to refactor this. I seem to remember that Chris worked on this new
>> feature so you may want to converse with him about its implementation.
>>
>> Adrian.
>>
>> Venkat Reddy wrote:
>>> Hi,
>>>
>>> AFPPaintingState is being used in three different places altogether
>>> in FopTrunk source. The default constructor is being used in the
>>> following three classes...
>>>
>>> 1. AFPDocumentHandler.java
>>> 2. AFPRenderer.java
>>> 3. AFPPageOverlayElement.java
>>>
>>> There is a variable 'resolution' is being initialized for each
>>> instance, this resolution parameter can be set using the 'fop.xconf'
>>> for a particular render...
>>>
>>> Ex:- AFPRenderer configuration below
>>>
>>> <renderer mime="application/x-afp">
>>> <!--
>>> The bit depth and type of images produced
>>> (this is the default setting)
>>> -->
>>> <images mode="b+w" bits-per-pixel="8"/>
>>> <renderer-resolution>1400</renderer-resolution>
>>>
>>>
>>> The above <renderer-resolution> is being hardcoded as '240dpi' in
>>> AFPRendererConfigurator.java, which initiates the renderer
>>> resolution based on the configuration set in 'fop.xconf'. In order
>>> to resolve this problem, I will be changing the AFPPaintingState as
>>> singleton, so that all the above classes will get the instance using
>>> 'getInstance()' method instead of default constructor. This will
>>> resolve the <renderer-resolution> problem as well, by a simple
>>> change in AFPRendererConfigurator (instead of hardcoded value 240,
>>> assigning the value from the configuration object).
>>>
>>> Please review the above changes and tell me, if I am doing anything
>>> wrong here?
>>>
>>> Thanks,
>>> Venkat.
>>>
>>
>>
>
>
Re: Making AFPPaintingState singleton
Posted by Venkat Reddy <va...@googlemail.com>.
Hi Adrian,
Thanks for your reply.
Yes, You are absolutely right, I am trying to handle these conversions
at the renderer level... I will soon come out with a patch to the bug
you opened.
Thanks once again for the source points,
Venkat.
Adrian Cumiskey wrote:
> Hi Venkat,
>
> This approach is not a good idea. It is possible that a runtime
> environment is configured to have multiple instances of Fop being
> instantiated by FopFactory with different rendering configurations
> (e.g. renderer resolution values).
>
> Its disappointing, but I've just noticed a bug in
> AFPPageOverlayElement, an AFPPaintingState shouldn't just be
> instantiated in there like that. The processNode implementation will
> not accurately calculate and plot the page overlay position if the
> document resolution is different from the detault value of 240dpi.
> This calculation should be carried out much later at rendering time,
> *not* in here at configuration time - its very hacky and you'll need
> to refactor this. I seem to remember that Chris worked on this new
> feature so you may want to converse with him about its implementation.
>
> Adrian.
>
> Venkat Reddy wrote:
>> Hi,
>>
>> AFPPaintingState is being used in three different places altogether
>> in FopTrunk source. The default constructor is being used in the
>> following three classes...
>>
>> 1. AFPDocumentHandler.java
>> 2. AFPRenderer.java
>> 3. AFPPageOverlayElement.java
>>
>> There is a variable 'resolution' is being initialized for each
>> instance, this resolution parameter can be set using the 'fop.xconf'
>> for a particular render...
>>
>> Ex:- AFPRenderer configuration below
>>
>> <renderer mime="application/x-afp">
>> <!--
>> The bit depth and type of images produced
>> (this is the default setting)
>> -->
>> <images mode="b+w" bits-per-pixel="8"/>
>> <renderer-resolution>1400</renderer-resolution>
>>
>>
>> The above <renderer-resolution> is being hardcoded as '240dpi' in
>> AFPRendererConfigurator.java, which initiates the renderer resolution
>> based on the configuration set in 'fop.xconf'. In order to resolve
>> this problem, I will be changing the AFPPaintingState as singleton,
>> so that all the above classes will get the instance using
>> 'getInstance()' method instead of default constructor. This will
>> resolve the <renderer-resolution> problem as well, by a simple change
>> in AFPRendererConfigurator (instead of hardcoded value 240, assigning
>> the value from the configuration object).
>>
>> Please review the above changes and tell me, if I am doing anything
>> wrong here?
>>
>> Thanks,
>> Venkat.
>>
>
>
Re: Making AFPPaintingState singleton
Posted by Adrian Cumiskey <de...@cumiskey.com>.
Hi Venkat,
This approach is not a good idea. It is possible that a runtime
environment is configured to have multiple instances of Fop being
instantiated by FopFactory with different rendering configurations (e.g.
renderer resolution values).
Its disappointing, but I've just noticed a bug in AFPPageOverlayElement,
an AFPPaintingState shouldn't just be instantiated in there like that.
The processNode implementation will not accurately calculate and plot
the page overlay position if the document resolution is different from
the detault value of 240dpi. This calculation should be carried out
much later at rendering time, *not* in here at configuration time - its
very hacky and you'll need to refactor this. I seem to remember that
Chris worked on this new feature so you may want to converse with him
about its implementation.
Adrian.
Venkat Reddy wrote:
> Hi,
>
> AFPPaintingState is being used in three different places altogether in
> FopTrunk source. The default constructor is being used in the
> following three classes...
>
> 1. AFPDocumentHandler.java
> 2. AFPRenderer.java
> 3. AFPPageOverlayElement.java
>
> There is a variable 'resolution' is being initialized for each
> instance, this resolution parameter can be set using the 'fop.xconf'
> for a particular render...
>
> Ex:- AFPRenderer configuration below
>
> <renderer mime="application/x-afp">
> <!--
> The bit depth and type of images produced
> (this is the default setting)
> -->
> <images mode="b+w" bits-per-pixel="8"/>
> <renderer-resolution>1400</renderer-resolution>
>
>
> The above <renderer-resolution> is being hardcoded as '240dpi' in
> AFPRendererConfigurator.java, which initiates the renderer resolution
> based on the configuration set in 'fop.xconf'. In order to resolve
> this problem, I will be changing the AFPPaintingState as singleton, so
> that all the above classes will get the instance using 'getInstance()'
> method instead of default constructor. This will resolve the
> <renderer-resolution> problem as well, by a simple change in
> AFPRendererConfigurator (instead of hardcoded value 240, assigning the
> value from the configuration object).
>
> Please review the above changes and tell me, if I am doing anything
> wrong here?
>
> Thanks,
> Venkat.
>