2018-04-12 16:47 GMT+08:00 Huw Davies <huw@codeweavers.com>:

On Thu, Apr 12, 2018 at 11:31:29AM +0800, Jactry Zeng wrote:
> Signed-off-by: Jactry Zeng <jzeng@codeweavers.com>
> ---
>�� dlls/riched20/caret.c�� | 30 ++++++++++++++++++++++++++++--
>�� dlls/riched20/editor.c�� |�� 1 +
>�� dlls/riched20/editstr.h |�� 8 +++++++-
>�� dlls/riched20/list.c�� |�� 8 ++++++--
>�� dlls/riched20/richole.c | 20 ++++++++++----------
>�� dlls/riched20/run.c�� |�� 2 +-
>�� dlls/riched20/writer.c�� |�� 6 +++---
>�� 7 files changed, 56 insertions(+), 19 deletions(-)
>
>

> diff --git a/dlls/riched20/caret.c b/dlls/riched20/caret.c
> index d676a1bc38..1e654a0a80 100644
> --- a/dlls/riched20/caret.c
> +++ b/dlls/riched20/caret.c
> @@ -471,6 +471,14 @@ ME_InternalInsertTextFromCursor(ME_TextEditor *editor, int nCursor,
>�� return ME_InsertRunAtCursor(editor, p, style, str, len, flags);
>�� }
>��
> +static ME_ReObjItem* create_reobj_item(const REOBJECT* reo)
> +{
> +�� ME_ReObjItem *reo_item = heap_alloc(sizeof(ME_ReObjItem));

�� sizeof(*reo_item) like the one you do on the next line.

Failure test.

> +
> +�� reo_item->reobj = heap_alloc(sizeof(*reo));

Having two allocs here is needless.�� embed the REOBJECT structure
inside ME_ReObjItem.

> +�� ME_CopyReObject(reo_item->reobj, reo);
> +�� return reo_item;
> +}
>��
>�� void ME_InsertOLEFromCursor(ME_TextEditor *editor, const REOBJECT* reo, int nCursor)
>�� {
> @@ -484,8 +492,26 @@ void ME_InsertOLEFromCursor(ME_TextEditor *editor, const REOBJECT* reo, int nCur
>��
>�� di = ME_InternalInsertTextFromCursor(editor, nCursor, &space, 1, pStyle,
>�� MERF_GRAPHICS);
> -�� di->member.run.ole_obj = heap_alloc(sizeof(*reo));
> -�� ME_CopyReObject(di->member.run.ole_obj, reo);
> +�� di->member.run.reo_item = create_reobj_item(reo);
> +
> +�� if (list_empty(&editor->reobj_list))
> +�� list_add_head(&editor->reobj_list, &di->member.run.reo_item->entry);

I don't think this is a useful optimization.�� It's probably also hiding
a bug below.

> +�� else
> +�� {
> +�� ME_DisplayItem *di_prev = di;
> +�� ME_ReObjItem *reo_item_prev = NULL;
> +
> +�� while (ME_PrevRun(NULL, &di_prev, TRUE))
> +�� {
> +�� if (di_prev->member.run.reo_item)
> +�� {
> +�� reo_item_prev = di_prev->member.run.reo_item;
> +�� break;
> +�� }
> +�� }

At this point reo_item_prev could be NULL, if it would be the first
item in the list.

> +�� list_add_after(&reo_item_prev->entry, &di->member.run.reo_item->entry);
> +�� }
> +
>�� ME_ReleaseStyle(pStyle);
>�� }
>��
> diff --git a/dlls/riched20/editor.c b/dlls/riched20/editor.c
> index c4425127e7..c57d3ea9b9 100644
> --- a/dlls/riched20/editor.c
> +++ b/dlls/riched20/editor.c
> @@ -3113,6 +3113,7 @@ ME_TextEditor *ME_MakeEditor(ITextHost *texthost, BOOL bEmulateVersion10)
>�� ed->wheel_remain = 0;
>��
>�� list_init( &ed->style_list );
> +�� list_init( &ed->reobj_list );
>�� OleInitialize(NULL);
>��
>�� return ed;
> diff --git a/dlls/riched20/editstr.h b/dlls/riched20/editstr.h
> index a37c4de46e..b32c2622c6 100644
> --- a/dlls/riched20/editstr.h
> +++ b/dlls/riched20/editstr.h
> @@ -153,6 +153,11 @@ typedef enum {
>��
>�� struct tagME_DisplayItem;
>��
> +typedef struct tagME_ReObjItem {
> +�� struct list entry;
> +�� REOBJECT *reobj;

As already suggested this become REOBJECT reobj

> +} ME_ReObjItem;

Let's drop the ME_ noise and the typedef and go for:

struct re_object
{
�� struct list entry;
�� REOBJECT obj;
};

>�� typedef struct tagME_Run
>�� {
>�� ME_Style *style;
> @@ -163,7 +168,7 @@ typedef struct tagME_Run
>�� int nFlags;
>�� int nAscent, nDescent; /* pixels above/below baseline */
>�� POINT pt; /* relative to para's position */
> -�� REOBJECT *ole_obj; /* FIXME: should be a union with strText (at least) */
> +�� ME_ReObjItem *reo_item; /* FIXME: should be a union with strText (at least) */
>��
>�� SCRIPT_ANALYSIS script_analysis;
>�� int num_glyphs, max_glyphs;
> @@ -436,6 +441,7 @@ typedef struct tagME_TextEditor
>�� BOOL bMouseCaptured;
>�� int wheel_remain;
>�� struct list style_list;
> +�� struct list reobj_list;
>�� } ME_TextEditor;
>��
>�� typedef struct tagME_Context
> diff --git a/dlls/riched20/list.c b/dlls/riched20/list.c
> index 58b64e8081..f7fb1e94b6 100644
> --- a/dlls/riched20/list.c
> +++ b/dlls/riched20/list.c
> @@ -97,7 +97,7 @@ BOOL ME_PrevRun(ME_DisplayItem **para, ME_DisplayItem **run, BOOL all_para)
>�� {
>�� if (p->type == diParagraph) {
>�� if (!all_para) return FALSE;
> -�� if (p->member.para.prev_para->type == diParagraph)
> +�� if (para && p->member.para.prev_para->type == diParagraph)

Why are you sneaking this change in here?�� It should be a separate patch.

>�� *para = p->member.para.prev_para;
>�� } else if (p->type == diRun) {
>�� *run = p;
> @@ -169,7 +169,11 @@ void ME_DestroyDisplayItem(ME_DisplayItem *item)
>��
>�� if (item->type==diRun)
>�� {
> -�� if (item->member.run.ole_obj) ME_DeleteReObject(item->member.run.ole_obj);
> +�� if (item->member.run.reo_item)
> +�� {
> +�� list_remove(&item->member.run.reo_item->entry);
> +�� ME_DeleteReObject(item->member.run.reo_item->reobj);
> +�� }
>�� heap_free( item->member.run.glyphs );
>�� heap_free( item->member.run.clusters );
>�� ME_ReleaseStyle(item->member.run.style);
> diff --git a/dlls/riched20/richole.c b/dlls/riched20/richole.c
> index c19a2e7ffa..d315cb9877 100644
> --- a/dlls/riched20/richole.c
> +++ b/dlls/riched20/richole.c
> @@ -5284,11 +5284,11 @@ void ME_GetOLEObjectSize(const ME_Context *c, ME_Run *run, SIZE *pSize)
>�� ENHMETAHEADER emh;
>��
>�� assert(run->nFlags & MERF_GRAPHICS);
> -�� assert(run->ole_obj);
> +�� assert(run->reo_item);
>��
> -�� if (run->ole_obj->sizel.cx != 0 || run->ole_obj->sizel.cy != 0)
> +�� if (run->reo_item->reobj->sizel.cx != 0 || run->reo_item->reobj->sizel.cy != 0)
>�� {
> -�� convert_sizel(c, &run->ole_obj->sizel, pSize);
> +�� convert_sizel(c, &run->reo_item->reobj->sizel, pSize);
>�� if (c->editor->nZoomNumerator != 0)
>�� {
>�� pSize->cx = MulDiv(pSize->cx, c->editor->nZoomNumerator, c->editor->nZoomDenominator);
> @@ -5297,13 +5297,13 @@ void ME_GetOLEObjectSize(const ME_Context *c, ME_Run *run, SIZE *pSize)
>�� return;
>�� }
>��
> -�� if (!run->ole_obj->poleobj)
> +�� if (!run->reo_item->reobj->poleobj)
>�� {
>�� pSize->cx = pSize->cy = 0;
>�� return;
>�� }
>��
> -�� if (IOleObject_QueryInterface(run->ole_obj->poleobj, &IID_IDataObject, (void**)&ido) != S_OK)
> +�� if (IOleObject_QueryInterface(run->reo_item->reobj->poleobj, &IID_IDataObject, (void**)&ido) != S_OK)
>�� {
>�� FIXME("Query Interface IID_IDataObject failed!\n");
>�� pSize->cx = pSize->cy = 0;
> @@ -5366,13 +5366,13 @@ void ME_DrawOLE(ME_Context *c, int x, int y, ME_Run *run, BOOL selected)
>�� RECT�� rc;
>��
>�� assert(run->nFlags & MERF_GRAPHICS);
> -�� assert(run->ole_obj);
> -�� if (IOleObject_QueryInterface(run->ole_obj->poleobj, &IID_IDataObject, (void**)&ido) != S_OK)
> +�� assert(run->reo_item);
> +�� if (IOleObject_QueryInterface(run->reo_item->reobj->poleobj, &IID_IDataObject, (void**)&ido) != S_OK)
>�� {
>�� FIXME("Couldn't get interface\n");
>�� return;
>�� }
> -�� has_size = run->ole_obj->sizel.cx != 0 || run->ole_obj->sizel.cy != 0;
> +�� has_size = run->reo_item->reobj->sizel.cx != 0 || run->reo_item->reobj->sizel.cy != 0;
>�� fmt.cfFormat = CF_BITMAP;
>�� fmt.ptd = NULL;
>�� fmt.dwAspect = DVASPECT_CONTENT;
> @@ -5399,7 +5399,7 @@ void ME_DrawOLE(ME_Context *c, int x, int y, ME_Run *run, BOOL selected)
>�� old_bm = SelectObject(hMemDC, stgm.u.hBitmap);
>�� if (has_size)
>�� {
> -�� convert_sizel(c, &run->ole_obj->sizel, &sz);
> +�� convert_sizel(c, &run->reo_item->reobj->sizel, &sz);
>�� } else {
>�� sz.cx = dibsect.dsBm.bmWidth;
>�� sz.cy = dibsect.dsBm.bmHeight;
> @@ -5419,7 +5419,7 @@ void ME_DrawOLE(ME_Context *c, int x, int y, ME_Run *run, BOOL selected)
>�� GetEnhMetaFileHeader(stgm.u.hEnhMetaFile, sizeof(emh), &emh);
>�� if (has_size)
>�� {
> -�� convert_sizel(c, &run->ole_obj->sizel, &sz);
> +�� convert_sizel(c, &run->reo_item->reobj->sizel, &sz);
>�� } else {
>�� sz.cx = emh.rclBounds.right - emh.rclBounds.left;
>�� sz.cy = emh.rclBounds.bottom - emh.rclBounds.top;
> diff --git a/dlls/riched20/run.c b/dlls/riched20/run.c
> index 098c4f8b4d..1cf96b54c8 100644
> --- a/dlls/riched20/run.c
> +++ b/dlls/riched20/run.c
> @@ -290,7 +290,7 @@ ME_DisplayItem *ME_MakeRun(ME_Style *s, int nFlags)
>�� {
>�� ME_DisplayItem *item = ME_MakeDI(diRun);
>�� item->member.run.style = s;
> -�� item->member.run.ole_obj = NULL;
> +�� item->member.run.reo_item = NULL;
>�� item->member.run.nFlags = nFlags;
>�� item->member.run.nCharOfs = -1;
>�� item->member.run.len = 0;
> diff --git a/dlls/riched20/writer.c b/dlls/riched20/writer.c
> index aad2e44879..7f170d20bc 100644
> --- a/dlls/riched20/writer.c
> +++ b/dlls/riched20/writer.c
> @@ -947,7 +947,7 @@ static BOOL stream_out_graphics( ME_TextEditor *editor, ME_OutStream *stream,
>�� SIZE goal, pic;
>�� ME_Context c;
>��
> -�� hr = IOleObject_QueryInterface( run->ole_obj->poleobj, &IID_IDataObject, (void **)&data );
> +�� hr = IOleObject_QueryInterface( run->reo_item->reobj->poleobj, &IID_IDataObject, (void **)&data );
>�� if (FAILED(hr)) return FALSE;
>��
>�� ME_InitContext( &c, editor, ITextHost_TxGetDC( editor->texthost ) );
> @@ -975,8 +975,8 @@ static BOOL stream_out_graphics( ME_TextEditor *editor, ME_OutStream *stream,
>�� emf_bits->szlMillimeters.cy * c.dpi.cy * 10 );
>��
>�� /* convert goal size to twips */
> -�� goal.cx = MulDiv( run->ole_obj->sizel.cx, 144, 254 );
> -�� goal.cy = MulDiv( run->ole_obj->sizel.cy, 144, 254 );
> +�� goal.cx = MulDiv( run->reo_item->reobj->sizel.cx, 144, 254 );
> +�� goal.cy = MulDiv( run->reo_item->reobj->sizel.cy, 144, 254 );
>��
>�� if (!ME_StreamOutPrint( stream, "{\\*\\shppict{\\pict\\emfblip\\picw%d\\pich%d\\picwgoal%d\\pichgoal%d\n",
>�� pic.cx, pic.cy, goal.cx, goal.cy ))
>

>

Regards,

Jactry Zeng