Bug 39395

Summary: PATCH: Placeholder shape and other improvements
Product: POI Reporter: Yegor Kozlov <yegor>
Component: HSLFAssignee: POI Developers List <dev>
Status: RESOLVED FIXED    
Severity: normal    
Priority: P2    
Version: unspecified   
Target Milestone: ---   
Hardware: Other   
OS: other   
Attachments: source code
patch with the changes

Description Yegor Kozlov 2006-04-24 14:44:42 UTC
(1) Obsolete classes which go away:
org.apache.poi.hslf.model.Ellipse.java
org.apache.poi.hslf.model.Rectangle.java

(2) TextBox didn't properly handle text in placeholders. 
Now the problem is fixed. 
Placeholder is a new subclass of TextBox which handles such text.
I also had to implement a couple of new records:

OEPlaceholderAtom.java
 - Describes a placeholder. 

OutlineTextRefAtom.java
 - If present it indicates that TextHeaderAtom, TextBytesAtom and StyleTextPropAtom
are stored in Document.SlideListWithText instead of the shape'S escher container.
The content is just a 0-based index of the [TextHeaderAtom, TextBytesAtom and
StyleTextPropAtom] block in SlideListWithText.

If you iterate over text shapes in a slide and collect them in a set 
it must be the same as returned by Slide.getTextRuns().
The test code is in TestShapes.testTextBoxSet() . It is a good sanity check
which revealed some bugs.

(4) Placeholder shape. It is a subclass of TextBox which stores text attributes
outside of it's escher container.

For now I can create the only type of placeholder: slide title.
Use case:

 Slide slide = ppt.createSlide(); //blank slide

 TextBox title = slide.addTitle(); //add title placeholder. 
 title.setText("Hello, World");

...

 If you edit this shape in powerpoint you can
  (a) see that it is a placeholder
  (b) see that it holds slide's title. Outline view shows the  correct title text .

I also added the sample code in how-to-shapes.xml

(5) Added getFontColor and setFontColor to RichTextRun. Now I think all
character format properties are covered.

Regards, Yegor Kozlov
Comment 1 Yegor Kozlov 2006-04-24 14:45:21 UTC
Created attachment 18167 [details]
source code
Comment 2 Yegor Kozlov 2006-04-24 14:45:47 UTC
Created attachment 18168 [details]
patch with the changes
Comment 3 Nick Burch 2006-05-08 16:42:24 UTC
Cheers for this, I've applied it. Couple of things though:

Do you think have a nice helper for RGB for the text colour (and presumably also
background colour, shape colour etc), splitting into the tripplets, or should we
just stick with using the java.awt one?

Looking at TextBox, I'm not happy with how much of RichTextRun it seems to be
duplicating. Can we just provide a RichTextRun over the top of it? Otherwise, we
should refactor both of them to remove the duplication.

Also, why did you change TextCharsAtom to no longer use the common StringUtil
stuff? I didn't commit that, I didn't see the point!
Comment 4 Yegor Kozlov 2006-05-10 07:30:23 UTC

>> Do you think have a nice helper for RGB for the text colour (and presumably also
>> background colour, shape colour etc), splitting into the tripplets, or should we
>> just stick with using the java.awt one?

End-user should work with  java.awt.Color. For internal purposes we
can use either approach, but what we have now is OK to me.

If you are going to work with colors in PowerPoint, be aware it is not always
logical:

- RGB values are swapped, i.e. it is 'BGR' instead of 'RGB'
- alpha component is 254 for rich text runs and 0 for shape color.

There may be more 'features' we don't know about .

>> Looking at TextBox, I'm not happy with how much of RichTextRun it seems to be
>> duplicating. Can we just provide a RichTextRun over the top of it? Otherwise, we
>> should refactor both of them to remove the duplication.

This is only temporary architecture. I have plans how to refactor it,
just need time to put things in order in my head and start
implementing it.


>> Also, why did you change TextCharsAtom to no longer use the common StringUtil
>> stuff? I didn't commit that, I didn't see the point!

Ops. My bad, sorry.
This is experimental code and it came to the patch by mistake.

I studied why TextBox didn't display extended ASCII wingdings and
decided to use TextCharsAtom by default, not TextBytesAtom when a new
TextBox is created. Unfortunately I couldn't do it with current TextCharsAtom.

The following simple code results in error:

        TextCharsAtom tca = new TextCharsAtom();
        String txt = tca.getText();

ava.lang.ArrayIndexOutOfBoundsException: Illegal offset
        at org.apache.poi.util.StringUtil.getFromUnicodeLE(StringUtil.java:67)
        at org.apache.poi.util.StringUtil.getFromUnicodeLE(StringUtil.java:91)
        at org.apache.poi.hslf.record.TextCharsAtom.getText(TextCharsAtom.java:44)

I looked at TextCharsAtom and simplified it a bit: construct UTF-16LE
string directly from a byte array. With this simplification it worked
fine.

And what were your reasons to use StringUtil instead of
constructing strings from byte array? I prefer to keep things simple.

Of course I would have discussed this change with you if I meant to commit it.

Regards, Yegor
Comment 5 Nick Burch 2006-05-10 13:24:35 UTC
(In reply to comment #4)
> >> Do you think have a nice helper for RGB for the text colour (and presumably
also
> >> background colour, shape colour etc), splitting into the tripplets, or
should we
> >> just stick with using the java.awt one?
> 
> End-user should work with  java.awt.Color. For internal purposes we
> can use either approach, but what we have now is OK to me.
> 
> If you are going to work with colors in PowerPoint, be aware it is not always
> logical:
> 
> - RGB values are swapped, i.e. it is 'BGR' instead of 'RGB'
> - alpha component is 254 for rich text runs and 0 for shape color.
> 
> There may be more 'features' we don't know about .

Hmm. Perhaps we will need our own wrapper longer term then, to handle these fun
features!

> >> Looking at TextBox, I'm not happy with how much of RichTextRun it seems to be
> >> duplicating. Can we just provide a RichTextRun over the top of it?
Otherwise, we
> >> should refactor both of them to remove the duplication.
> 
> This is only temporary architecture. I have plans how to refactor it,
> just need time to put things in order in my head and start
> implementing it.

OK, sure. We should probably warn people that the API is subject to a lot of
change on it though!

> >> Also, why did you change TextCharsAtom to no longer use the common StringUtil
> >> stuff? I didn't commit that, I didn't see the point!
> 
> Ops. My bad, sorry.
> This is experimental code and it came to the patch by mistake.
> 
> I studied why TextBox didn't display extended ASCII wingdings and
> decided to use TextCharsAtom by default, not TextBytesAtom when a new
> TextBox is created. Unfortunately I couldn't do it with current TextCharsAtom.

I've fixed StringUtils so it now works, and added a test for the TextCharsAtom
constructor stuff

> And what were your reasons to use StringUtil instead of
> constructing strings from byte array? I prefer to keep things simple.

I prefer not to duplicate code, when we already have handy util methods :)

Nick