Hi! As you may see I've fixed the generate branch to compile (that is good).
But still there are some problems that arised and needs to be fixed:
* The GP_SET_BITS() used in GP_PutPixel_Raw doesn't work correctly unless you explicitly cast the result from GP_PIXEL_ADDR_32BPP() to (uint32_t*) (and same for each pixel > 8BPP).
* The gfx part for now uses GP_PutPixel_Raw() (which is wrong because we need to clip every pixel (at least for the more complicated shapes). So variant without transformation but with clipping is needed too.
* The HLine for 1, 2 and 4 BPP for different bit endians is not written at all.
* The GP_PutPixel and GP_GetPixel now uses transformation flags for default but the rest of the library doesn't (eg. gfx part). Should we simply rename GP_TLine to GP_Line and GP_Line to GP_Line_Raw and so?
And that's it for now, I guess.
Hi!
- The gfx part for now uses GP_PutPixel_Raw() (which is wrong because we need to clip every pixel (at least for the more complicated shapes). So variant without transformation but with clipping is needed too.
Or better we should settle for subcontext and clipp ony for context boundaries. Shouldn't the Raw function clip at least for these?
Hi,
- The gfx part for now uses GP_PutPixel_Raw() (which is wrong because
we need to clip every pixel (at least for the more complicated shapes). So variant without transformation but with clipping is needed too.
Or better we should settle for subcontext and clipp ony for context boundaries. Shouldn't the Raw function clip at least for these?
I am in favor of subcontexts. However, I would expect Raw to be as raw(=fast) as possible. There are quite a few use-cases where the algorithm takes care of clipping (e.g. line drawing).
We can easily have both versions. The biggest cost being consistency and confusion. What about the following: * "normal" functions respect both transformations and clipping (subcontext boundaries) * "Raw" functions ignore transformations but clip on subcontext boundary * "RawNoclip" (or similar, explicit name) ignore everything. The name should explicitely indicate that danger. We will probably only have get/putpixel and maybe v/hline in this category.
Tomas
Hi!
- The gfx part for now uses GP_PutPixel_Raw() (which is wrong because
we need to clip every pixel (at least for the more complicated shapes). So variant without transformation but with clipping is needed too.
Or better we should settle for subcontext and clipp ony for context boundaries. Shouldn't the Raw function clip at least for these?
I am in favor of subcontexts. However, I would expect Raw to be as raw(=fast) as possible. There are quite a few use-cases where the algorithm takes care of clipping (e.g. line drawing).
Hmm now we just need someone to get the subcontextes implemented, what was the problem with that? I remeber waguely there were some problems with not byte aligned pixel types.
Yes, at least for vertical lines it's quite easy to clip at the start (and we may have for example two alorithms for circle one with clipping and one without as figuring out if circle crosses some border is easy...)
We can easily have both versions. The biggest cost being consistency and confusion. What about the following:
- "normal" functions respect both transformations and clipping
(subcontext boundaries)
- "Raw" functions ignore transformations but clip on subcontext boundary
- "RawNoclip" (or similar, explicit name) ignore everything. The name
should explicitely indicate that danger. We will probably only have get/putpixel and maybe v/hline in this category.
Okay that sounds reasonable.
Hi,
- The gfx part for now uses GP_PutPixel_Raw() (which is wrong because
we need to clip every pixel (at least for the more complicated shapes). So variant without transformation but with clipping is needed too.
Or better we should settle for subcontext and clipp ony for context boundaries. Shouldn't the Raw function clip at least for these?
I am in favor of subcontexts. However, I would expect Raw to be as raw(=fast) as possible. There are quite a few use-cases where the algorithm takes care of clipping (e.g. line drawing).
Hmm now we just need someone to get the subcontextes implemented, what was the problem with that? I remeber waguely there were some problems with not byte aligned pixel types.
For >=8bpp types, you can just set the base pointer to the new "top-left" corner and change the size preserving bytesperrow. Voila - subcontext. The only problem were subpixel types - there the pionter would also need to have "pixel offset". Per-bpp functions can ignore it altogether for >=8bpp, but it still creates some mess. Both "normal" and "Raw" functions accessing the context should do this "shifting". Should it be done in PutPixel or GP_PIXEL_ADDR/OFFSET (more general and probably right, but even messier)?
I will see whether I manage to include that into generate (my time-schedule is very stretched for few weeks now), but feel free to add subcontexts for >=8bpp for now. It may be wise to remove the old clipping at the same time. This would disable changing <8bpp clipping but that may be ok for now (for generate).
Tomas
Hi!
Hmm now we just need someone to get the subcontextes implemented, what was the problem with that? I remeber waguely there were some problems with not byte aligned pixel types.
For >=8bpp types, you can just set the base pointer to the new "top-left" corner and change the size preserving bytesperrow. Voila - subcontext.
The only problem were subpixel types - there the pionter would also need to have "pixel offset". Per-bpp functions can ignore it altogether for >=8bpp, but it still creates some mess. Both "normal" and "Raw" functions accessing the context should do this "shifting". Should it be done in PutPixel or GP_PIXEL_ADDR/OFFSET (more general and probably right, but even messier)?
Well I know you hate to hear that, but there are really > 8BPP byte unaligned pixel types 19BPP for example (which is supported by linux on arm targets and yes I have such hardware). But the solution should be the same as for subbyte pixels.
I will see whether I manage to include that into generate (my time-schedule is very stretched for few weeks now), but feel free to add subcontexts for >=8bpp for now. It may be wise to remove the old clipping at the same time. This would disable changing <8bpp clipping but that may be ok for now (for generate).
Okay, I'll do the preparations and function to create subcontext for byte-aligned pixels.
Hi,
On Mon, Jul 18, 2011 at 1:37 PM, Cyril Hrubis metan@ucw.cz wrote:
Hi!
Hmm now we just need someone to get the subcontextes implemented, what was the problem with that? I remeber waguely there were some problems with not byte aligned pixel types.
For >=8bpp types, you can just set the base pointer to the new "top-left" corner and change the size preserving bytesperrow. Voila - subcontext.
The only problem were subpixel types - there the pionter would also need to have "pixel offset". Per-bpp functions can ignore it altogether for >=8bpp, but it still creates some mess. Both "normal" and "Raw" functions accessing the context should do this "shifting". Should it be done in PutPixel or GP_PIXEL_ADDR/OFFSET (more general and probably right, but even messier)?
Well I know you hate to hear that, but there are really > 8BPP byte unaligned pixel types 19BPP for example (which is supported by linux on arm targets and yes I have such hardware). But the solution should be the same as for subbyte pixels.
"Hate" is not right. If there is such a word, I would hesitate to write it down.
[with curiosity replacing loathing for a moment] How is 19bpp used/divided? And what hardware? Is it on 19bit Harvard-type processors supporting ternary arithmetic while driving on the left side of the road?
I will see whether I manage to include that into generate (my time-schedule is very stretched for few weeks now), but feel free to add subcontexts for >=8bpp for now. It may be wise to remove the old clipping at the same time. This would disable changing <8bpp clipping but that may be ok for now (for generate).
Okay, I'll do the preparations and function to create subcontext for byte-aligned pixels.
Thanks, Tomas
Hi!
The only problem were subpixel types - there the pionter would also need to have "pixel offset". Per-bpp functions can ignore it altogether for >=8bpp, but it still creates some mess. Both "normal" and "Raw" functions accessing the context should do this "shifting". Should it be done in PutPixel or GP_PIXEL_ADDR/OFFSET (more general and probably right, but even messier)?
Well I know you hate to hear that, but there are really > 8BPP byte unaligned pixel types 19BPP for example (which is supported by linux on arm targets and yes I have such hardware). But the solution should be the same as for subbyte pixels.
"Hate" is not right. If there is such a word, I would hesitate to write it down.
[with curiosity replacing loathing for a moment] How is 19bpp used/divided? And what hardware? Is it on 19bit Harvard-type processors supporting ternary arithmetic while driving on the left side of the road?
Not really that strange; PXA ARM processors supports 19BPP and 18BPP hardware displays. The display lines (at least) are byte aligned. The kernel framebuffer then gives you buffer to write to, which is, for example, 19BPP. And such displays are not that uncommon on ARM machines.
Hi,
Great job!
On Mon, Jul 18, 2011 at 11:29 AM, Cyril Hrubis metan@ucw.cz wrote:
Hi! As you may see I've fixed the generate branch to compile (that is good).
Thanks for fixing the rest of the library. Were there bigger problems in core?
But still there are some problems that arised and needs to be fixed:
- The GP_SET_BITS() used in GP_PutPixel_Raw doesn't work correctly
unless you explicitly cast the result from GP_PIXEL_ADDR_32BPP() to (uint32_t*) (and same for each pixel > 8BPP).
Uhh ... right. GP_SET_BITS is meant to be general and respects the parameter type size. Thanks.
- The GP_PutPixel and GP_GetPixel now uses transformation flags for default
but the rest of the library doesn't (eg. gfx part). Should we simply rename GP_TLine to GP_Line and GP_Line to GP_Line_Raw and so?
I would be in favor of that. The user should not need it and there are always the Raw variants.
Thanks again, Tomas
Hi!
As you may see I've fixed the generate branch to compile (that is good).
Thanks for fixing the rest of the library. Were there bigger problems in core?
Not really, besides the GP_SET_BITS() only small things like offset for RGB colors backwards (compared to how X server needed them for drawing).
But still there are some problems that arised and needs to be fixed:
- The GP_SET_BITS() used in GP_PutPixel_Raw doesn't work correctly
unless you explicitly cast the result from GP_PIXEL_ADDR_32BPP() to (uint32_t*) (and same for each pixel > 8BPP).
Uhh ... right. GP_SET_BITS is meant to be general and respects the parameter type size. Thanks.
Would you pretty please fix that (I would rather leave that to you).
- The GP_PutPixel and GP_GetPixel now uses transformation flags for default
but the rest of the library doesn't (eg. gfx part). Should we simply rename GP_TLine to GP_Line and GP_Line to GP_Line_Raw and so?
I would be in favor of that. The user should not need it and there are always the Raw variants.
Okay, will do.
Hi,
2011/7/18 Cyril Hrubis metan@ucw.cz:
Hi!
As you may see I've fixed the generate branch to compile (that is good).
Thanks for fixing the rest of the library. Were there bigger problems in core?
Not really, besides the GP_SET_BITS() only small things like offset for RGB colors backwards (compared to how X server needed them for drawing).
You mean in pixeltype defs?
But still there are some problems that arised and needs to be fixed:
- The GP_SET_BITS() used in GP_PutPixel_Raw doesn't work correctly
unless you explicitly cast the result from GP_PIXEL_ADDR_32BPP() to (uint32_t*) (and same for each pixel > 8BPP).
Uhh ... right. GP_SET_BITS is meant to be general and respects the parameter type size. Thanks.
Would you pretty please fix that (I would rather leave that to you).
I would leave GP_SET_BITS as is and add the explicit cast. In my TODO.
Why is GP_Context.pixels of type "uint8_t *"? Wouldn't "void *" make more sense? (they are generally not individual bytes after all). This would help catch this bug (as "typeof(void)1") and future similar ones.
- The GP_PutPixel and GP_GetPixel now uses transformation flags for default
but the rest of the library doesn't (eg. gfx part). Should we simply rename GP_TLine to GP_Line and GP_Line to GP_Line_Raw and so?
I would be in favor of that. The user should not need it and there are always the Raw variants.
Okay, will do.
Great. What about the Raw/RawNoclip distinction?
Tomas
Hi!
As you may see I've fixed the generate branch to compile (that is good).
Thanks for fixing the rest of the library. Were there bigger problems in core?
Not really, besides the GP_SET_BITS() only small things like offset for RGB colors backwards (compared to how X server needed them for drawing).
You mean in pixeltype defs?
Yep, looks like the most usual variant is XRGB with X the most signigicant byte and B the least.
But still there are some problems that arised and needs to be fixed:
- The GP_SET_BITS() used in GP_PutPixel_Raw doesn't work correctly
unless you explicitly cast the result from GP_PIXEL_ADDR_32BPP() to (uint32_t*) (and same for each pixel > 8BPP).
Uhh ... right. GP_SET_BITS is meant to be general and respects the parameter type size. Thanks.
Would you pretty please fix that (I would rather leave that to you).
I would leave GP_SET_BITS as is and add the explicit cast. In my TODO.
Why is GP_Context.pixels of type "uint8_t *"? Wouldn't "void *" make more sense? (they are generally not individual bytes after all). This would help catch this bug (as "typeof(void)1") and future similar ones.
We may change that, but then you need to cast it explicitly in some macros that expect it to be pointer bytes (in GNU C void* is actually byte addresable). And it's kind of common sense to reffer to buffer as buffer of bytes.
So my conclusion is change that if you want, but fix the surrounding code.
- The GP_PutPixel and GP_GetPixel now uses transformation flags for default
but the rest of the library doesn't (eg. gfx part). Should we simply rename GP_TLine to GP_Line and GP_Line to GP_Line_Raw and so?
I would be in favor of that. The user should not need it and there are always the Raw variants.
Okay, will do.
Great. What about the Raw/RawNoclip distinction?
Needs to be settled down, but having each drawing algorithm in Raw and RawNoClip variant seems to me like quite duplication.
Hi,
I would leave GP_SET_BITS as is and add the explicit cast. In my TODO.
Why is GP_Context.pixels of type "uint8_t *"? Wouldn't "void *" make more sense? (they are generally not individual bytes after all). This would help catch this bug (as "typeof(void)1") and future similar ones.
We may change that, but then you need to cast it explicitly in some macros that expect it to be pointer bytes (in GNU C void* is actually byte addresable). And it's kind of common sense to reffer to buffer as buffer of bytes.
So my conclusion is change that if you want, but fix the surrounding code.
I will think about it. Any other solution you would suggest? (As generated GP_SET_PIXEL must be changed anyway to remove the overflow warnings) I would like to keep GP_SET_PIXEL general, but another variant would be a viable option.
- The GP_PutPixel and GP_GetPixel now uses transformation flags for default
but the rest of the library doesn't (eg. gfx part). Should we simply rename GP_TLine to GP_Line and GP_Line to GP_Line_Raw and so?
I would be in favor of that. The user should not need it and there are always the Raw variants.
Okay, will do.
Great. What about the Raw/RawNoclip distinction?
Needs to be settled down, but having each drawing algorithm in Raw and RawNoClip variant seems to me like quite duplication.
I meant RawNoclip to exist only for very low-level operations (get/putpixel, h/vline). Drawing function can then decide whether it should take care of clipping itself (easy with line), leave clipping to putpixel (any complicated shapes) or do something in-between (very optional, e.g. your smart circle example).
Just as a side-remark: most drawing functions may want to do some crude clipping to avoid too many calls to (clipped) putpixel.
Tomas
Hi!
We may change that, but then you need to cast it explicitly in some macros that expect it to be pointer bytes (in GNU C void* is actually byte addresable). And it's kind of common sense to reffer to buffer as buffer of bytes.
So my conclusion is change that if you want, but fix the surrounding code.
I will think about it. Any other solution you would suggest? (As generated GP_SET_PIXEL must be changed anyway to remove the overflow warnings) I would like to keep GP_SET_PIXEL general, but another variant would be a viable option.
I wouldn't be against a little more straightforward code eg. less nested macros but that is up to you.
Great. What about the Raw/RawNoclip distinction?
Needs to be settled down, but having each drawing algorithm in Raw and RawNoClip variant seems to me like quite duplication.
I meant RawNoclip to exist only for very low-level operations (get/putpixel, h/vline). Drawing function can then decide whether it should take care of clipping itself (easy with line), leave clipping to putpixel (any complicated shapes) or do something in-between (very optional, e.g. your smart circle example).
Just as a side-remark: most drawing functions may want to do some crude clipping to avoid too many calls to (clipped) putpixel.
Sounds reasonable.