On 15 Nov 2017, at 09:36, Ben Coman <btc@openInWorld.com> wrote:




On 15 November 2017 at 17:27, Esteban Lorenzano <estebanlm@gmail.com> wrote:


> On 15 Nov 2017, at 02:05, Ben Coman <btc@openInWorld.com> wrote:
>
> What is the recommended way for a C basic type to be passed-by-reference to function wanting to use it for output.
> For example 'width' & 'height' here in this library unction...
>
>   int FPDF_GetPageSizeByIndex(FPDF_DOCUMENT document,
>                           int page_index,
>                           double* width,
>                           double* height);
>
> void mytestGetPageSizeByIndex(doc) {
>            double width, height;
>             FPDF_GetPageSizeByIndex(doc, 0, &width, &height);
>             printf("width=%f, height=%f\n", width, height);
> }
>
> gives...
>    width=200.000000, height=200.000000
>
>
>
> In Pharo I'm trying...
>
>   FFIOpaqueObject subclass: #FPDF_DOCUMENT
>
>   FPDF_GetPageSizeByIndex__document: document
>                           page_index: page_index
>                           width: width
>                           height: height
>     ^self ffiCall: #(int FPDF_GetPageSizeByIndex(
>                                               FPDF_DOCUMENT *document,
>                                               int page_index,
>                                               FFIFloat64 * width,
>                                               FFIFloat64 * height))
>
>
> testGetPageSizeByIndex
>       | document page_index width height result|
>       PDFium FPDF_InitLibrary.
>       document := PDFium FPDF_LoadDocument__file_path:  helloPdf  password: ''.
>       width := 0.0.
>       height := 0.0.
>       page_index := 0. "Its zero based"
>       result := PDFium FPDF_GetPageSizeByIndex__document: document
>                        page_index: 0
>                        width: width
>                        height: height.
>       PDFium FPDF_CloseDocument__document: document.
>       PDFium FPDF_DestroyLibrary.
>       self assert: document isNull not. "Document opened okay, and btw this works for a different pageCount test"
>       self assert: result > 0.  "Non-zero for success. 0 for error (document or page not found)"
>       self halt.
>
>
> and at the halt the Inspector shows...
> result = 1
> width = 0.0
> height = 0.0

no, that will not work :)
what you need to do here is to pass a ���buffer��� to contain the width and height:

testGetPageSizeByIndex
        | document page_index widthBuffer heightBuffer width height result|
        PDFium FPDF_InitLibrary.
        document := PDFium FPDF_LoadDocument__file_path:  helloPdf  password: ''.
        widthBuffer := ByteArray new: (FFIFloat64 typeSize).
        heightBuffer := ByteArray new: (FFIFloat64 typeSize).

Thanks Esteban. That worked. However I needed a minor tweak...
   widthBuffer := ByteArray new: (FFIFloat64 new typeSize).
or...
   widthBuffer := ByteArray new: (FFIFloat64 externalTypeSize).


Now it would be nice to do...
   widthBuffer := FFIFloat64 newBuffer.

you know, I thought the same :)
but since I���m officially on holiday I didn���t do it (yet). 

If you want to add an issue and send a PR, it would be perfect :)


which could be implemented...
   FFIExternalType class >> newBuffer.
        ^ByteArray new: (self externalTypeSize)

testGetPageSizeByIndex
        | document page_index widthBuffer heightBuffer width height result|
        PDFium FPDF_InitLibrary.
        document := PDFium FPDF_LoadDocument__file_path:  helloPdf  password: ''.
        widthBuffer := FFIFloat64 buffer.
        heightBuffer := FFIFloat64 buffer.
        page_index := 0. "Its zero based"
        result := PDFium FPDF_GetPageSizeByIndex__document: document
                         page_index: 0
                         width: widthBuffer
                         height: heightBuffer.
        width := widthBuffer doubleAt: 1.
        height := heightBuffer doubleAt: 1.



btw, a broader curiousity even though this is likely set in stone and not even your personal choice - why "FFIFloat64" and not "FFIDouble" ?
   widthBuffer := FFIDouble newBuffer.
would be a nice match to the callout declaration.

because the ABI defines float64 as the right name, heh.
and yes, it was not my personal choice ;)


Would it be overkill to have in Pharo an empty FFIDouble subclassed from FFIFloat64 ? 
the superclass being explicit about the format, while the subclass provides a nice identifier for users?

maybe it is overkilling. Because also��� where do you put the limit? (doubles, floats, longs, etc��� ?) 

Esteban


 
        page_index := 0. "Its zero based"
        result := PDFium FPDF_GetPageSizeByIndex__document: document
                         page_index: 0
                         width: widthBuffer
                         height: heightBuffer.
        width := widthBuffer doubleAt: 1.
        height := heightBuffer doubleAt: 1. 
        PDFium FPDF_CloseDocument__document: document.
        PDFium FPDF_DestroyLibrary.
        self assert: document isNull not. "Document opened okay, and btw this works for a different pageCount test"
        self assert: result > 0.  "Non-zero for success. 0 for error (document or page not found)"
        self halt.

side note: I do not recommend using the UFFI type in declarations. In the long way is worst for comprehension and maintainability. This thingy:

^self ffiCall: #(int FPDF_GetPageSizeByIndex(
                                                FPDF_DOCUMENT *document,
                                                int page_index,
                                                FFIFloat64 * width,
                                                FFIFloat64 * height))

would be better as in original version:

^self ffiCall: #(int FPDF_GetPageSizeByIndex(
                                                FPDF_DOCUMENT *document,
                                                int page_index,
                                                double * width,
                                                double * height))

Thanks for the heads up.

cheers -ben