#371 closed enhancement (fixed)
psPixels
| Reported by: | Owned by: | Paul Price | |
|---|---|---|---|
| Priority: | high | Milestone: | |
| Component: | PSLib SDRS | Version: | unspecified |
| Severity: | minor | Keywords: | |
| Cc: |
Description
Possible enhancement:
Currently, psPixels is defined merely as two psVectors, one for x, and one as y
coordinates.
To efficiently qsort the pixels (as required for psPixelsConcatenate), I found
it advantageous to transfer the data into an array of x,y pair structs, i.e.,
typedef struct {
psS32 x;
psS32 y;
} pixelCoord;
...
pixelCoord coordinates[length];
...
This prompts the question, would it not be better if psPixels had the same
structure? I.e.,
typedef struct {
int n;
int nalloc;
psPixelCoord* data;
} psPixels;
This would eliminate many things that I otherwise have to deal with, including
size mismatches and multiple type-support of the psVectors in the old
definition, It would also allow a qsort in-place, eliminating the need for a
shadow buffer for concatenation, etc.
Another option is to modify psVector (or psArray) to have support for x,y pairs
by adding the pixelCoords struct in the data union, and use that instead of
psPixels.
-rdd
Change History (9)
comment:1 by , 21 years ago
| Resolution: | → fixed |
|---|---|
| Status: | new → closed |
comment:2 by , 21 years ago
| Resolution: | fixed |
|---|---|
| Status: | closed → reopened |
Well, dynamically allocating the individual psPixelCoords struct is not really
what I was suggesting, as all these small structs would be extremely memory
inefficient. If you expect the list to be very small, it probably is just fine,
but if it is of a significant size, that would create a significant number of
small blocks with significant overhead.
What is was thinking was something more in the line of psVector, where there is
a single dynamic buffer holding the array of structs. Basically, a psVector of
a pair of psS32s (x,y). All the basic psVector functions could be defined for
the x,y pair, but if it is not desirable to have it in psVector, the psPixels
struct could duplicate the basic psVector structure, i.e.,
typedef struct {
int n;
const int nalloc;
psPixelCoord* data;
} psPixels;
I could make the psArray of psPixelCoords work, but I suspect that would not be
quite as user-friendly as above.
-rdd
comment:3 by , 21 years ago
I don't think it would be a good idea to have this in psVector --- this doesn't
strike me as a math type, but really just a storage type.
So, how's this:
typedef struct {
int x; x coordinate
int y; y coordinate
} psPixelCoord;
typedef struct {
int n; Number in use
const int nalloc; Number allocated
psPixelCoord *data; The pixel coordinates
} psPixels;
comment:4 by , 21 years ago
| Resolution: | → fixed |
|---|---|
| Status: | reopened → closed |
Yes, that is basically what I had tried to propose. It is psVector-like, as it
is accessed in the same manner, but not integrated into psVector. I think I
confused things up by giving two options (a psVector-like psPixels vs.
integrating the functionality into psVector), but not delimitating between them
well.
If it looks good to you, Let's go with the last definition of psPixelCoord and
psPixels you gave.
-rdd
comment:5 by , 21 years ago
No, your proposal was fine; I'm just daft.
Updated SDRS (psLib and Modules).
comment:6 by , 21 years ago
FYI: I added a psPixelsAlloc, psPixelsRealloc, and psPixelsCopy as well, just to
complete the package.
-rdd
comment:7 by , 21 years ago
Added psPixelsAlloc and psPixelsRealloc to the SDRS.
I think the functionality of psPixelsCopy is provided by psPixelsConcatenate
(correct me if I'm wrong).
comment:8 by , 21 years ago
If the first psPixels* parameter is NULL, psPixelsConcatenate calls
psPixelCopy. This functionality is pulled out to a separate function to better
parallelize the psVector function-set, hence to increase usability and
continuity.
comment:9 by , 21 years ago
Good idea.
\begin{verbatim}
psPixels *psPixelsCopy(psPixels *out, const psPixels *pixels);
psPixels *psPixelsConcatenate(psPixels *out, const psPixels *pixels);
\end{verbatim}
\code{psPixelsCopy} shall copy the contents of \code{pixels} to the
\code{out}. In the event that \code{out} is \code{NULL}, a new
\code{psPixels} shall be allocated, and the contents of \code{pixels}
simply copied in. If \code{pixels} is \code{NULL}, the function shall
generate an error and return \code{NULL}.
\code{psPixelsConcatenate} shall concatenate the \code{pixels} onto
\code{out}. In the event that \code{out} is \code{NULL}, the function
performs a \code{psPixelsCopy}, returning the copy. If \code{pixels}
is \code{NULL}, the function shall generate an error and return
\code{NULL}. The function shall take care to ensure that there are no
duplicate pixels in \code{out} (since the order in which the pixels
are stored is not important, the values may be sorted, allowing the
use of a faster algorithm than a linear scan).

This sounds good.
typedef struct {
} psPixelCoord;
I changed each "psPixels *whatever" to a "psArray *whatever", where the array
holds psPixelCoords.