Bug 48002 - [PATCH] AFP plot values are incorrectly calculated in page overlays when renderer-resolution setting not 240dpi
Summary: [PATCH] AFP plot values are incorrectly calculated in page overlays when rend...
Status: NEW
Alias: None
Product: Fop - Now in Jira
Classification: Unclassified
Component: general (show other bugs)
Version: trunk
Hardware: PC Linux
: P2 normal
Target Milestone: ---
Assignee: fop-dev
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-10-15 09:33 UTC by Adrian Cumiskey
Modified: 2020-02-09 20:20 UTC (History)
0 users



Attachments
Page Overlay patch (4.06 KB, patch)
2009-10-19 06:57 UTC, Venkat Reddy
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Adrian Cumiskey 2009-10-15 09:33:27 UTC
Plot values for AFP page overlays are always calculated at a resolution of 240dpi regardless of configuration settings.  This calculation should be done at rendering time - not during configuration (See http://markmail.org/search/?q=fop-dev#query:fop-dev%20order%3Adate-backward+page:1+mid:6xesm7i7jkwiozjg+state:results) for details.

Adrian.
Comment 1 Venkat Reddy 2009-10-19 06:57:44 UTC
Created attachment 24395 [details]
Page Overlay patch

Please some one review my patch...
Comment 2 Venkat Reddy 2009-10-19 07:00:32 UTC
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.
Comment 3 Venkat Reddy 2009-10-19 07:07:48 UTC
As per the Adrian comments above, I have prepared a patch for this bug with the changes to the following classes:

AFPRendererConfigurator.java
=======================================

customizable.setResolution(rendererResolutionCfg.getValueAsInteger()); //previously 240 is hard coded here

AFPPageOverlayElement.java
==========================================
Removed the AFPPaintingState usage in this file(unit conversion from mpts to units)

if (AFPElementMapping.INCLUDE_PAGE_OVERLAY.equals(elementName)) {
            // convert user specific units to mpts and set the coordinates for the page overlay
            pageOverlay.setX(UnitConv.convert(attlist.getValue(ATT_X)));
            pageOverlay.setY(UnitConv.convert(attlist.getValue(ATT_Y)));
        }

AFPRenderer.java
===============================================
Included the conversion process in here, AFPPaintingState object is already availble here

int x = (int)unitConv.mpt2units(ipo.getX());
                                int y = (int)unitConv.mpt2units(ipo.getY());
                                dataStream.createIncludePageOverlay(overlay, x, y);

AFPDocumentHandler.java
================================================
Similar chage to above...

int x = (int)unitConv.mpt2units(ipo.getX());
                                int y = (int)unitConv.mpt2units(ipo.getY());
                                dataStream.createIncludePageOverlay(overlay, x, y);

Please someone review my patch attached to this bug...


Thanks,
Venkat.
Comment 4 Glenn Adams 2012-04-07 01:41:23 UTC
resetting P2 open bugs to P3 pending further review
Comment 5 Glenn Adams 2012-04-11 03:20:19 UTC
increase priority for bugs with a patch