Printing doesn't work in ShowImage

[jackburton]

If you try to print from ShowImage, the application will hang. A spool file will be created, but the print_server doesn't seem to recognize it. Printing (somewhat) works from StyledEdit and Terminal. Component=Showimage for now, but could be a print_server or libbe issue. Feel free to redirect as needed.

[jackburton]

I tracked down the problem: ShowImage hangs in BPrintJob::DrawView, which itself calls BPicture::Flatten(). Flatten() downloads the picture from the app_server. In this case the picture, being a bitmap, is bigger than most normal pictures (in my test, the Haiku logo - big version), it's 4.825.808 bytes). Adding a Flush() to the BPortLink link, in an intermediate position in the app_server code (ServerPicture::ExportData(), right after StartMessage(B_OK)) fixes the problem (it flushes the link before it gets too big). So I assume this being a problem in the BPortLink implementation.

[jackburton]

That said, I agree it's not a good idea to pass that much data through ports. Maybe we should add some code to portlink which automatically uses areas when the data exceeds a certain size ?

[jackburton]

A doubt just stroke my mind: Can portlink be used like I do in ServerPicture::ExportData() vs Picture::_Download() ? In ExportData I do multiple Attach() while in _Download() I do one big Read().

Is this correct ?

[jackburton]

I added some checks, and can confirm that we reach the maximum limit for the buffer for LinkSender.

[jackburton]

Replying to jackburton:

I added some checks, and can confirm that we reach the maximum limit for the buffer for LinkSender.

What we'd need is something like the original portlink implementation in beos: it didn't send whole messages, but a stream of data: so big chunks of data could be sent splitted through the port, and the client would do reads in a loop until the requested size would be fulfilled.

[laplace]

This is a quite serious bug for printing, I propose to change its milestone to R1/alpha1.

[jackburton]

Replying to laplace:

This is a quite serious bug for printing, I propose to change its milestone to R1/alpha1.

I agree, but I don't think I have the time to work on this, sorry. So someone else will have to fix it. Currently, except for what I wrote in comment 6, I don't have a good idea on how to solve this.

[laplace]

CCed Karsten as he brought up this topic on printing mailing list.

In R5 BView seems to support a kind of "disk mode" SetDiskMode(filename, offset) according to the comment in our View.cpp the picture is flattened to the file while the drawing commands are recorded, maybe that could be utilized, if SetDiskMode(...) would be implemented ;)?

On the other hand this might be an optimization only and un-/flattening of large bitmaps must be supported anyway?

[julun]

Hi,

can we do a partial upload in _Upload(), like splitting the data in a defined size and use link.Attach<void*> and link.Read<void*> as long all data are up in the app_server? Same for download? Would that work? This would effect both _Upload()/ _Download in BPicture and ImportData/ExportData in ServerPicture, right? If that could work i can give it a try next week, I'm off over the weekend.

Regards, Karsten

[julun]

Forget what i wrote, i had a short look at ServerLink.Attach/ Read. Need to think about it again.

[julun]

Hi,

I've created an version of LinkSender/Receiver using an area to pass data arround the exceeds the max buffer size. I'm not sure if i leak the area in LinkSender. If someone has the time, please review.

Thanks Karsten

[laplace]

In line 126 and 131 of your patch you have removed the space after "for" statement, and inside the space after each semicolon is also missing.

Why have you removed "new (std::nothrow)" in line 123, 124. This will leak opList, if an exception is thrown when allocation of ptList fails? Either that or if exceptions are disabled, a null pointer access could happen, if memory allocation did fail.

Maybe you could use class AutoDeleter (or whatever it is called) to delete the buffers automatically that are for example allocated in line 976 or 1014.

Otherwise it looks good as far as I can tell, besides in rare cases the area could be leaked on receiver side, if the receiver dies before it can delete the area.

In order to fix that, the sender side somehow needs to keep track of the opened areas and delete them after the receiver has died. BTW is the sender == app_server? The sender also has to be notified, when the receiver deletes the area, so that the sender does not delete it twice. Could be a problem, if the area_id has be reused for a new area. I don't know how likely that is.

In case of bad performance an area cache might be necessary too?

Does printing from ShowImage work now, BTW?

[julun]

Hi,

I'm sorry for the confusing patch. i made the diff with an not up to date version of my tree. I will add a new one ASAP, also without those whitespaces for easyer review. With this patch printing works fine from ShowImage.

Karsten

[julun]

Hi,

here is an updated one, I don't know why the first one showed all those strange removal stuff. Hope this one is easyer to review. Thanks for the comments so far. :)

Karsten

[jackburton]

Ah, yes! That looks very nice, indeed! Moreover, done this way, it will also solve other problems related to big transfers between app_server and libbe (just check Window.cpp). Good work!

[jackburton]

One more comment, besides what Michael already wrote: since resize_area() can fail, you should check its return value too. And in case it happens, I guess you could try to delete and re-create the area. Thanks for your work, btw!

[jackburton]

Replying to jackburton:

Ah, yes! That looks very nice, indeed! Moreover, done this way, it will also solve other problems related to big transfers between app_server and libbe (just check Window.cpp).

Sorry, it's in View.cpp (for example, BView::FillShape())

[stippi]

The new patch looks good to me, only you need to use new (std::nothrow) in the first part of the patch (two places) and check the returned pointer. Return B_NO_MEMORY if it's NULL.

[julun]

Hi all,

thanks for the comments. I have integrated them so far and reduced a bit of the code also. If you agree on that patch i will commit it later today.

Karsten

[stippi]

You are leaking the first buffer in the error case. Looks fine otherwise, but I kind of looked in a hurry... :-)

[julun]

Hi,

thanks, i fixed it and commited in hrev27214.

Karsten