The SDL forums have moved to discourse.libsdl.org.
This is just a read-only archive of the previous forums, to keep old links working.

Ok, so I was just running a small program that moved variously sized bubbles around the screen.

But here's the problem.

When I do a scaled blit such that the destination rectange is partly out of bounds...the blit is squished to fit inside the dest rectangle, instead of clipped so that the parts out of bounds are simply sliced off.


This causes my bubbles to go squish when they bounce off the wall, instead of partly floating off the screen/window.


I was also told by someone in #sdl on freenode that the renderer does what I expect by scaling before clipping, so finding that 


Suggested fix:  use the original dest_rect to determine the intended dimensions of the blit, but only use clipping to decide how much of it to do.


I'd also like to point out that doing a non scaled blit clips as expected, and you'd think that a BlitScaled with identity scaling would do the same thing.


SDL version: 2.0.3

Is this using SDL_RenderCopy()?

Jonny D




On Mon, Aug 24, 2015 at 3:35 PM, Raymond Jennings wrote:

No, using SDL_BlitScaled.

Though I did also try using SDL_RenderCopyEx and that does what I expected it to do.  SDL_BlitScaled does not.


On Mon, Aug 24, 2015 at 1:35 PM, Jonathan Dearborn wrote:

What's the advantage of SDL_BlitSurface ?

I think I might have some patch for that floating around on bugzilla. The software renderer had some issues due to that behaviour IIRC. The bug number escapes me and I won't have access to a computer for a few days.

--
Melker Narikka

Raymond Jennings kirjoitti ma elokuuta 24 23:45:34 2015 GMT+0300:
_______________________________________________
SDL mailing list

http://lists.libsdl.org/listinfo.cgi/sdl-libsdl.org

I assume then that the proper behavior of BlitScaled is to match what RenderCopyEx does?

On Mon, Aug 24, 2015 at 3:09 PM, Melker Narikka wrote:

It doesn't really have an advantage over SDL_RenderCopyEx, but I think it should still work correctly.

Somehow I get the impression that this is going to be an automatic wontfix because of that.


On Mon, Aug 24, 2015 at 3:04 PM, AntTheAlchemist wrote:

I don't know if this is a bug, but it may be undocumented behaviour.
I think it's preferred that you clip your source surface/texture first to get the desired behaviour.
Then again, is this actually undesirable?  This may be a bit if a larger conversation. On 24 Aug 2015 6:15 pm, "Raymond Jennings" wrote:

On Mon, 24 Aug 2015 22:09:50 +0000
Melker Narikka wrote:


https://bugzilla.libsdl.org/show_bug.cgi?id=1968

I guess you might be thinking of that one.

I'll take a stab at it, although I recall last time I tried it wasn't
as straight-forward as I hoped. Might be wrong. If anyone has a patch
for this, I'd be happy to test it.

--
driedfruit
_______________________________________________
SDL mailing list

http://lists.libsdl.org/listinfo.cgi/sdl-libsdl.org

2015-08-25 8:35 GMT-03:00, Alex Barry:

I definitely wouldn't expect it to clamp coordinates instead of
clipping, every other drawing function performs clipping and pretty
much every other drawing API ever does the same.
_______________________________________________
SDL mailing list

http://lists.libsdl.org/listinfo.cgi/sdl-libsdl.org

https://bugzilla.libsdl.org/show_bug.cgi?id=2646
https://bugzilla.libsdl.org/attachment.cgi?id=1777&action=diff

The attachment linked above has some clipping magic in it that might be adaptable to BlitScaled. Apparently I opted to handle it in the software renderer instead.

--
Melker Narikka

Driedfruit kirjoitti ma elokuuta 24 04:44:51 2015 GMT+0300:
_______________________________________________
SDL mailing list

http://lists.libsdl.org/listinfo.cgi/sdl-libsdl.org

On Mon, 24 Aug 2015 22:09:50 +0000
Melker Narikka wrote:


https://bugzilla.libsdl.org/show_bug.cgi?id=1968

OK, I gave it a go and it doesn't look too good. If you have a
better patch, I'd gladly test it. Here's what I got so far. The
problem is two-fold:

the source coords for the blit might be incorrect due to rounding
errors around SDL_surface.c:745


It's quite possible to get 0 when it "should've" been 1, or hit some
other edge case.

The second problem is in SDL_SoftStretch, which takes 2 rects to
perform a blit; it determines both the blit area AND the scaling
factors from those rects; the rects are already clipped against each
other, and thus the scaling factors are always calculated incorrectly.

The scaling factors, instead, should be calculated before the clipping
is done, on original rects. There's no good interface to then pass
those to SDL_SoftStrech, so the patch below takes the wrong approach
and breaks the API/ABI. But that's beside the point.

More to the point, the result is still wrong about 25% of the cases, as
I haven't fully grokked the copy_* functions of SDL_stretch.c. In any
case the patch is very bad, and is more of an invitation to a
discussion, than a real solution.


diff -r 6436a20efcae include/SDL_surface.h
--- a/include/SDL_surface.h Mon Aug 24 22:35:01 2015 +0200
+++ b/include/SDL_surface.h Thu Aug 25 22:18:25 2016 +0400
@@ -471,7 +471,7 @@
extern DECLSPEC int SDLCALL SDL_SoftStretch(SDL_Surface * src,
const SDL_Rect * srcrect,
SDL_Surface * dst,
- const SDL_Rect * dstrect);
+ const SDL_Rect * dstrect,
int sw, int sh);
#define SDL_BlitScaled SDL_UpperBlitScaled

@@ -489,7 +489,7 @@
*/
extern DECLSPEC int SDLCALL SDL_LowerBlitScaled
(SDL_Surface * src, SDL_Rect * srcrect,
- SDL_Surface * dst, SDL_Rect * dstrect);
+ SDL_Surface * dst, SDL_Rect * dstrect, int w, int h);


/* Ends C function definitions when using C++ */
diff -r 6436a20efcae src/dynapi/SDL_dynapi_procs.h
--- a/src/dynapi/SDL_dynapi_procs.h Mon Aug 24 22:35:01 2015
+0200 +++ b/src/dynapi/SDL_dynapi_procs.h Thu Aug 25 22:18:25
2016 +0400 @@ -500,9 +500,9 @@
SDL_DYNAPI_PROC(int,SDL_FillRects,(SDL_Surface *a, const SDL_Rect *b,
int c, Uint32 d),(a,b,c,d),return)
SDL_DYNAPI_PROC(int,SDL_UpperBlit,(SDL_Surface *a, const SDL_Rect *b,
SDL_Surface *c, SDL_Rect *d),(a,b,c,d),return)
SDL_DYNAPI_PROC(int,SDL_LowerBlit,(SDL_Surface *a, SDL_Rect *b,
SDL_Surface *c, SDL_Rect *d),(a,b,c,d),return)
-SDL_DYNAPI_PROC(int,SDL_SoftStretch,(SDL_Surface *a, const SDL_Rect
*b, SDL_Surface *c, const SDL_Rect *d),(a,b,c,d),return)
+SDL_DYNAPI_PROC(int,SDL_SoftStretch,(SDL_Surface *a, const SDL_Rect
*b, SDL_Surface *c, const SDL_Rect *d, int e, int
f),(a,b,c,d,e,f),return)
SDL_DYNAPI_PROC(int,SDL_UpperBlitScaled,(SDL_Surface *a, const SDL_Rect
*b, SDL_Surface *c, SDL_Rect *d),(a,b,c,d),return)
-SDL_DYNAPI_PROC(int,SDL_LowerBlitScaled,(SDL_Surface *a, SDL_Rect *b,
SDL_Surface *c, SDL_Rect *d),(a,b,c,d),return)
+SDL_DYNAPI_PROC(int,SDL_LowerBlitScaled,(SDL_Surface *a, SDL_Rect *b,
SDL_Surface *c, SDL_Rect *d, int e, int f),(a,b,c,d,e,f),return)
SDL_DYNAPI_PROC(SDL_bool,SDL_GetWindowWMInfo,(SDL_Window *a,
SDL_SysWMinfo *b),(a,b),return) SDL_DYNAPI_PROC(const
char*,SDL_GetThreadName,(SDL_Thread *a),(a),return)
SDL_DYNAPI_PROC(SDL_threadID,SDL_ThreadID,(void),(),return) diff -r
6436a20efcae src/render/SDL_yuv_sw.c ---
a/src/render/SDL_yuv_sw.c Mon Aug 24 22:35:01 2015 +0200 +++
b/src/render/SDL_yuv_sw.c Thu Aug 25 22:18:25 2016 +0400 @@
-1384,7 +1384,7 @@ } if (stretch) { SDL_Rect rect = *srcrect;
- SDL_SoftStretch(swdata->stretch, &rect, swdata->display, NULL);
+ SDL_SoftStretch(swdata->stretch, &rect, swdata->display, NULL,
0, 0); }
return 0;
}
diff -r 6436a20efcae src/video/SDL_stretch.c
--- a/src/video/SDL_stretch.c Mon Aug 24 22:35:01 2015 +0200
+++ b/src/video/SDL_stretch.c Thu Aug 25 22:18:25 2016 +0400
@@ -147,7 +147,7 @@
}

#endif /* USE_ASM_STRETCH */
-
+int global_stretch_inc_factor = 0;
#define DEFINE_COPY_ROW(name, type) \
static void name(type *src, int src_w, type *dst, int dst_w) \
{ \
@@ -156,7 +156,7 @@
type pixel = 0; \
\
pos = 0x10000; \
- inc = (src_w << 16) / dst_w; \
+ inc = global_stretch_inc_factor; \
for ( i=dst_w; i>0; --i ) { \
while ( pos >= 0x10000L ) { \
pixel = *src++; \
@@ -182,6 +182,7 @@

pos = 0x10000;
inc = (src_w << 16) / dst_w;
+ inc = global_stretch_inc_factor;
for (i = dst_w; i > 0; --i) {
while (pos >= 0x10000L) {
pixel[0] = *src++;
@@ -201,7 +202,8 @@
*/
int
SDL_SoftStretch(SDL_Surface * src, const SDL_Rect * srcrect,
- SDL_Surface * dst, const SDL_Rect * dstrect)
+ SDL_Surface * dst, const SDL_Rect * dstrect,
+ int scale_w, int scale_h)
{
int src_locked;
int dst_locked;
@@ -278,6 +280,8 @@
src_row = srcrect->y;
dst_row = dstrect->y;

+ if (scale_h) inc = scale_h;
+
#ifdef USE_ASM_STRETCH
/* Write the opcodes for this stretch */
if ((bpp == 3) || (generate_rowbytes(srcrect->w, dstrect->w, bpp)
< 0)) { @@ -295,6 +299,8 @@
++src_row;
pos -= 0x10000L;
}
+ global_stretch_inc_factor = (srcrect->w << 16) / dstrect->w;
+ if (scale_w) global_stretch_inc_factor = scale_w;
#ifdef USE_ASM_STRETCH
if (use_asm) {
#ifdef __GNUC__
diff -r 6436a20efcae src/video/SDL_surface.c
--- a/src/video/SDL_surface.c Mon Aug 24 22:35:01 2015 +0200
+++ b/src/video/SDL_surface.c Thu Aug 25 22:18:25 2016 +0400
@@ -628,6 +628,7 @@
double scaling_w, scaling_h;
int src_w, src_h;
int dst_w, dst_h;
+ int stretch_w, stretch_h;

/* Make sure the surfaces aren't locked */
if (!src || !dst) {
@@ -746,6 +747,14 @@
final_src.w = (int)SDL_floor(src_x1 - src_x0 + 1.5);
final_src.h = (int)SDL_floor(src_y1 - src_y0 + 1.5);

+ /* None of those make 100% sense... */
+ if (final_src.x + final_src.w >= src_w)
+ final_src.w = src_w - final_src.x;
+ //if (final_src.y + final_src.h >= src_h)
+ //final_src.h = src_h - final_src.y;
+ if (final_src.y + final_src.h >= src_h)
+ final_src.y = src_h - final_src.h;
+
final_dst.x = (int)SDL_floor(dst_x0 + 0.5);
final_dst.y = (int)SDL_floor(dst_y0 + 0.5);
final_dst.w = (int)SDL_floor(dst_x1 - dst_x0 + 1.5);
@@ -765,7 +774,13 @@
return 0;
}

- return SDL_LowerBlitScaled(src, &final_src, dst, &final_dst);
+ /* Calculate stretch factors based on *original* rects */
+ stretch_w = (src_w << 16) / dst_w;
+ stretch_h = (src_h << 16) / dst_h;
+
+ return SDL_LowerBlitScaled(src, &final_src, dst, &final_dst,
+ stretch_w, stretch_h
+ );
}

/**
@@ -774,7 +789,7 @@
*/
int
SDL_LowerBlitScaled(SDL_Surface * src, SDL_Rect * srcrect,
- SDL_Surface * dst, SDL_Rect * dstrect)
+ SDL_Surface * dst, SDL_Rect * dstrect, int sw, int sy)
{
static const Uint32 complex_copy_flags = (
SDL_COPY_MODULATE_COLOR | SDL_COPY_MODULATE_ALPHA |
@@ -790,7 +805,7 @@
if ( !(src->map->info.flags & complex_copy_flags) &&
src->format->format == dst->format->format &&
!SDL_ISPIXELFORMAT_INDEXED(src->format->format) ) {
- return SDL_SoftStretch( src, srcrect, dst, dstrect );
+ return SDL_SoftStretch( src, srcrect, dst, dstrect, sw, sy);
} else {
return SDL_LowerBlit( src, srcrect, dst, dstrect );
}


--
driedfruit
_______________________________________________
SDL mailing list

http://lists.libsdl.org/listinfo.cgi/sdl-libsdl.org

(Sorry for possible double-posting).

On Mon, 24 Aug 2015 22:09:50 +0000
Melker Narikka wrote:


https://bugzilla.libsdl.org/show_bug.cgi?id=1968

OK, I gave it a go and it doesn't look too good. If you have a
better patch, I'd gladly test it. Here's what I got so far. The
problem is two-fold:

the source coords for the blit might be incorrect due to rounding
errors around SDL_surface.c:745


It's quite possible to get 0 when it "should've" been 1, or hit some
other edge case.

The second problem is in SDL_SoftStretch, which takes 2 rects to
perform a blit; it determines both the blit area AND the scaling
factors from those rects; the rects are already clipped against each
other, and thus the scaling factors are always calculated incorrectly.

The scaling factors, instead, should be calculated before the clipping
is done, on original rects. There's no good interface to then pass
those to SDL_SoftStrech, so the patch below takes the wrong approach
and breaks the API/ABI. But that's beside the point.

More to the point, the result is still wrong about 25% of the cases, as
I haven't fully grokked the copy_* functions of SDL_stretch.c. In any
case the patch is very bad, and is more of an invitation to a
discussion, than a real solution.


diff -r 6436a20efcae include/SDL_surface.h
--- a/include/SDL_surface.h Mon Aug 24 22:35:01 2015 +0200
+++ b/include/SDL_surface.h Thu Aug 25 22:18:25 2016 +0400
@@ -471,7 +471,7 @@
extern DECLSPEC int SDLCALL SDL_SoftStretch(SDL_Surface * src,
const SDL_Rect * srcrect,
SDL_Surface * dst,
- const SDL_Rect * dstrect);
+ const SDL_Rect * dstrect,
int sw, int sh);
#define SDL_BlitScaled SDL_UpperBlitScaled

@@ -489,7 +489,7 @@
*/
extern DECLSPEC int SDLCALL SDL_LowerBlitScaled
(SDL_Surface * src, SDL_Rect * srcrect,
- SDL_Surface * dst, SDL_Rect * dstrect);
+ SDL_Surface * dst, SDL_Rect * dstrect, int w, int h);


/* Ends C function definitions when using C++ */
diff -r 6436a20efcae src/dynapi/SDL_dynapi_procs.h
--- a/src/dynapi/SDL_dynapi_procs.h Mon Aug 24 22:35:01 2015
+0200 +++ b/src/dynapi/SDL_dynapi_procs.h Thu Aug 25 22:18:25
2016 +0400 @@ -500,9 +500,9 @@
SDL_DYNAPI_PROC(int,SDL_FillRects,(SDL_Surface *a, const SDL_Rect *b,
int c, Uint32 d),(a,b,c,d),return)
SDL_DYNAPI_PROC(int,SDL_UpperBlit,(SDL_Surface *a, const SDL_Rect *b,
SDL_Surface *c, SDL_Rect *d),(a,b,c,d),return)
SDL_DYNAPI_PROC(int,SDL_LowerBlit,(SDL_Surface *a, SDL_Rect *b,
SDL_Surface *c, SDL_Rect *d),(a,b,c,d),return)
-SDL_DYNAPI_PROC(int,SDL_SoftStretch,(SDL_Surface *a, const SDL_Rect
*b, SDL_Surface *c, const SDL_Rect *d),(a,b,c,d),return)
+SDL_DYNAPI_PROC(int,SDL_SoftStretch,(SDL_Surface *a, const SDL_Rect
*b, SDL_Surface *c, const SDL_Rect *d, int e, int
f),(a,b,c,d,e,f),return)
SDL_DYNAPI_PROC(int,SDL_UpperBlitScaled,(SDL_Surface *a, const SDL_Rect
*b, SDL_Surface *c, SDL_Rect *d),(a,b,c,d),return)
-SDL_DYNAPI_PROC(int,SDL_LowerBlitScaled,(SDL_Surface *a, SDL_Rect *b,
SDL_Surface *c, SDL_Rect *d),(a,b,c,d),return)
+SDL_DYNAPI_PROC(int,SDL_LowerBlitScaled,(SDL_Surface *a, SDL_Rect *b,
SDL_Surface *c, SDL_Rect *d, int e, int f),(a,b,c,d,e,f),return)
SDL_DYNAPI_PROC(SDL_bool,SDL_GetWindowWMInfo,(SDL_Window *a,
SDL_SysWMinfo *b),(a,b),return) SDL_DYNAPI_PROC(const
char*,SDL_GetThreadName,(SDL_Thread *a),(a),return)
SDL_DYNAPI_PROC(SDL_threadID,SDL_ThreadID,(void),(),return) diff -r
6436a20efcae src/render/SDL_yuv_sw.c ---
a/src/render/SDL_yuv_sw.c Mon Aug 24 22:35:01 2015 +0200 +++
b/src/render/SDL_yuv_sw.c Thu Aug 25 22:18:25 2016 +0400 @@
-1384,7 +1384,7 @@ } if (stretch) { SDL_Rect rect = *srcrect;
- SDL_SoftStretch(swdata->stretch, &rect, swdata->display, NULL);
+ SDL_SoftStretch(swdata->stretch, &rect, swdata->display, NULL,
0, 0); }
return 0;
}
diff -r 6436a20efcae src/video/SDL_stretch.c
--- a/src/video/SDL_stretch.c Mon Aug 24 22:35:01 2015 +0200
+++ b/src/video/SDL_stretch.c Thu Aug 25 22:18:25 2016 +0400
@@ -147,7 +147,7 @@
}

#endif /* USE_ASM_STRETCH */
-
+int global_stretch_inc_factor = 0;
#define DEFINE_COPY_ROW(name, type) \
static void name(type *src, int src_w, type *dst, int dst_w) \
{ \
@@ -156,7 +156,7 @@
type pixel = 0; \
\
pos = 0x10000; \
- inc = (src_w << 16) / dst_w; \
+ inc = global_stretch_inc_factor; \
for ( i=dst_w; i>0; --i ) { \
while ( pos >= 0x10000L ) { \
pixel = *src++; \
@@ -182,6 +182,7 @@

pos = 0x10000;
inc = (src_w << 16) / dst_w;
+ inc = global_stretch_inc_factor;
for (i = dst_w; i > 0; --i) {
while (pos >= 0x10000L) {
pixel[0] = *src++;
@@ -201,7 +202,8 @@
*/
int
SDL_SoftStretch(SDL_Surface * src, const SDL_Rect * srcrect,
- SDL_Surface * dst, const SDL_Rect * dstrect)
+ SDL_Surface * dst, const SDL_Rect * dstrect,
+ int scale_w, int scale_h)
{
int src_locked;
int dst_locked;
@@ -278,6 +280,8 @@
src_row = srcrect->y;
dst_row = dstrect->y;

+ if (scale_h) inc = scale_h;
+
#ifdef USE_ASM_STRETCH
/* Write the opcodes for this stretch */
if ((bpp == 3) || (generate_rowbytes(srcrect->w, dstrect->w, bpp)
< 0)) { @@ -295,6 +299,8 @@
++src_row;
pos -= 0x10000L;
}
+ global_stretch_inc_factor = (srcrect->w << 16) / dstrect->w;
+ if (scale_w) global_stretch_inc_factor = scale_w;
#ifdef USE_ASM_STRETCH
if (use_asm) {
#ifdef __GNUC__
diff -r 6436a20efcae src/video/SDL_surface.c
--- a/src/video/SDL_surface.c Mon Aug 24 22:35:01 2015 +0200
+++ b/src/video/SDL_surface.c Thu Aug 25 22:18:25 2016 +0400
@@ -628,6 +628,7 @@
double scaling_w, scaling_h;
int src_w, src_h;
int dst_w, dst_h;
+ int stretch_w, stretch_h;

/* Make sure the surfaces aren't locked */
if (!src || !dst) {
@@ -746,6 +747,14 @@
final_src.w = (int)SDL_floor(src_x1 - src_x0 + 1.5);
final_src.h = (int)SDL_floor(src_y1 - src_y0 + 1.5);

+ /* None of those make 100% sense... */
+ if (final_src.x + final_src.w >= src_w)
+ final_src.w = src_w - final_src.x;
+ //if (final_src.y + final_src.h >= src_h)
+ //final_src.h = src_h - final_src.y;
+ if (final_src.y + final_src.h >= src_h)
+ final_src.y = src_h - final_src.h;
+
final_dst.x = (int)SDL_floor(dst_x0 + 0.5);
final_dst.y = (int)SDL_floor(dst_y0 + 0.5);
final_dst.w = (int)SDL_floor(dst_x1 - dst_x0 + 1.5);
@@ -765,7 +774,13 @@
return 0;
}

- return SDL_LowerBlitScaled(src, &final_src, dst, &final_dst);
+ /* Calculate stretch factors based on *original* rects */
+ stretch_w = (src_w << 16) / dst_w;
+ stretch_h = (src_h << 16) / dst_h;
+
+ return SDL_LowerBlitScaled(src, &final_src, dst, &final_dst,
+ stretch_w, stretch_h
+ );
}

/**
@@ -774,7 +789,7 @@
*/
int
SDL_LowerBlitScaled(SDL_Surface * src, SDL_Rect * srcrect,
- SDL_Surface * dst, SDL_Rect * dstrect)
+ SDL_Surface * dst, SDL_Rect * dstrect, int sw, int sy)
{
static const Uint32 complex_copy_flags = (
SDL_COPY_MODULATE_COLOR | SDL_COPY_MODULATE_ALPHA |
@@ -790,7 +805,7 @@
if ( !(src->map->info.flags & complex_copy_flags) &&
src->format->format == dst->format->format &&
!SDL_ISPIXELFORMAT_INDEXED(src->format->format) ) {
- return SDL_SoftStretch( src, srcrect, dst, dstrect );
+ return SDL_SoftStretch( src, srcrect, dst, dstrect, sw, sy);
} else {
return SDL_LowerBlit( src, srcrect, dst, dstrect );
}


--
driedfruit
_______________________________________________
SDL mailing list

http://lists.libsdl.org/listinfo.cgi/sdl-libsdl.org

Ugh, sorry, had some mail trouble, missed some posts.

I'll see if this clipping magic is better than the sorcery currently
in use.

On Tue, 25 Aug 2015 17:26:01 +0000
Melker Narikka wrote:

--
driedfruit
_______________________________________________
SDL mailing list

http://lists.libsdl.org/listinfo.cgi/sdl-libsdl.org

On Tue, 25 Aug 2015 09:44:11 -0300
Sik the hedgehog wrote:


SDL_Blit* is used behind the scenes in software renderer, so those two
indeed should behave the same. As for clipping vs clamping in general,


Yeah. Anything else is pretty unexpected.


--
driedfruit
_______________________________________________
SDL mailing list

http://lists.libsdl.org/listinfo.cgi/sdl-libsdl.org

2015-08-25 14:26 GMT-03:00, Melker Narikka:

I remember some time ago this bug being reported but only for the
software renderer, that may be why.
_______________________________________________
SDL mailing list

http://lists.libsdl.org/listinfo.cgi/sdl-libsdl.org

Actually I think that clipping first manually isn't even possible, because the position in the source texture corresponding to the edge/corner of the destination surface may not even be an integer!

Imagine if you're doing a scaled blit, but the right edge of the screen straddles the middle of a source pixel.


SDL_Rect uses ints, not floats, so this is impossible to get right manually.




On Tue, Aug 25, 2015 at 7:25 PM, Sik the hedgehog wrote: