(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
Created attachment 18167 [details] source code
Created attachment 18168 [details] patch with the changes
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!
>> 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
(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