Add support for the rectangular area operations #14112
Comments
j4james
added
the
Issue-Feature
ghost
added
Needs-Triage
|
@KalleOlaviNiemitalo and @jerch I believe you both have access to DEC terminals that should be capable of supporting these operations. So if you have the time, I'd be very grateful if you would be willing to run some tests which could help us figure out how exactly they are are supposed to work. I've create a Python script with a series of test cases here: For each test, it shows what I believe to be the expected output in the top of the screen, and the actual output in the bottom half. You just press enter to move on to the next test case until you've run through all of them (there are about 60 in total). With any luck, most of them will be correct, but if anything doesn't match, just make a note of the test number, and we can figure out where to go from there. If you want to go back and rerun an individual test, you can specify the test number as a command line parameter (e.g. |
|
@j4james, my current laptop lacks RS-232 ports. I may have a converter somewhere, though. |
|
@j4james Sure will see if I get down to it tomorrow. Also I kinda forgot about the other sequence you wanted to be checked several months ago. Can you give a pointer to that again? The reminder is kinda buried in tons of notifications now 😸 |
|
@jerch The other one is here: #13091 (comment) But there's no rush for any of this, so I don't want to stress either of you if you don't have the time, or don't have the necessary equipment. I was just thinking it would be good to have some evidence for how these sequences are really meant to work, and then maybe we could start pushing TEs to improve their interoperability - right now everyone interprets the standards differently. |
|
I'm hoping to get the |
carlos-zamora
added
Product-Conhost
|
@j4james thanks, this is a great idea. We'd love to have it! :) |
|
VT420 powers up OK. The USB serial converter should arrive on Tuesday. |
|
I got the converter, but now there is a gender problem. 😖 |
|
@KalleOlaviNiemitalo No pressure, but I got a bit overexcited by the idea of your converter arriving, and I started working on some other VT420 operations which I was hoping you might also be able to test. 😊 Not the end of the world if you can't get it working though, and there's no hurry for any of this. |
|
Our team's also recently come into possession of a VT420! We don't have nearly the right set of devices to interface with it yet, so I am more than happy to ask (beg!) @KalleOlaviNiemitalo to help out 😄 |
|
FYI, even if you can't get the terminal connected, you should still be able to do a certain amount of testing on it if you're desperate. Not the kind of test cases I linked above, but little one-off tests should be feasible. What you do is switch the terminal from On Line to Local mode (use F3 to open the setup menu, and you'll find that option on the Global submenu). Then you can simply type escape sequences to see what effect they have. When you need an And note that you can use the arrow keys to move the cursor around the screen, because the escape sequences that those keys generate are exactly the same as the output sequences for cursor movement. |
|
Serial I/O works. Now I just need the time. |
|
I attached all screen shots to https://gist.github.com/j4james/f8f3fb5ecd066dfc722dd98fec7a5742?permalink_comment_id=4334375#gistcomment-4334375. |
|
Is it possible to implement the rectangle stuff in such a way that it is compatible with the old terminals on ASCII characters, but also compatible with modern terminal emulators on Unicode grapheme clusters? |
That's an interesting question. I hadn't thought of trying that before, but I've done a few tests now on the terminals I have that support both rectangular area operations as well as Unicode. When you have a rectangular operation intersecting a glyph that spans two cells, that's much the same problem as when you output a single cell glyph on top of a two cell glyph. Most terminals just erase the whole glyph leaving a blank on one side or the other. Windows Terminal and conhost do things differently, and their exact behavior depends on the renderer in use, but I expect we'll fix that at some point in time. A similar situation arises when using The most interesting case though is the XTerm's behavior doesn't seem intentional though, because if you do something to trigger a refresh, it rerenders the rectangle occupying the expected dimensions (still displaying the full width characters). For example, if you requested a rectangle 10 cells wide, it starts off as 10 glyphs occupying 20 cells, but refreshes as 5 glyphs occupying 10 cells. Personally I think the latter rendering is the nicest, so it's a pity XTerm doesn't get that right by default. |
## Summary of the Pull Request This PR adds support for the rectangular area escape sequences: `DECCRA`, `DECFRA`, `DECERA`, `DECSERA`, `DECCARA`, `DECRARA`, and `DECSACE`. They provide VT applications with an efficient way to copy, fill, erase, or change the attributes in a rectangular area of the screen. ## PR Checklist * [x] Closes #14112 * [x] CLA signed. * [x] Tests added/passed * [ ] Documentation updated. * [ ] Schema updated. * [x] I've discussed this with core contributors already. Issue number where discussion took place: #14112 ## Detailed Description of the Pull Request / Additional comments All of these operations take a rectangle, defined by four coordinates. These need to have defaults applied, potentially need to be clipped and/or clamped within the active margins, and finally converted to absolute buffer coordinates. To avoid having to repeat that boilerplate code everywhere, I've pulled that functionality out into a shared method which they all use. With that out of the way, operations like `DECFRA` (fill), `DECERA` (erase), and `DECSERA` (selective erase) are fairly simple. They're just filling the given rectangle using the existing methods `_FillRect` and `_SelectiveEraseRect`. `DECCRA` (copy) is a little more work, because we didn't have existing code for that in `AdaptDispatch`, but it's mostly just cloned from the conhost `_CopyRectangle` function. The `DECCARA` (change attributes) and `DECRARA` (reverse attributes) operations are different though. Their coordinates can be interpreted as either a rectangle, or a stream of character positions (determined by the `DECSACE` escape sequence), and they both deal with attribute manipulation of the target area. So again I've pulled out that common functionality into some shared methods. They both also take a list of `SGR` options which define the attribute changes that they need to apply to the target area. To parse that data, I've had to refactor the `SGR` decoder from the `SetGraphicsRendition` method so it could be used with a given `TextAttribute` instance instead of just modifying the active attributes. The way that works in `DECCARA`, we apply the `SGR` options to two `TextAttribute` instances - one with all rendition bits on, and one with all off - producing a pair of bit masks. Then by `AND`ing the target attributes with the first bit mask, and `OR`ing them with the second, we can efficiently achieve the same effect as if we'd applied each `SGR` option to our target cells one by one. In the case of `DECRARA`, we only need to create a single bit mask to achieve the "reverse attribute" effect. That bit mask is applied to the target cells with an `XOR` operation. ## Validation Steps Performed Thanks to @KalleOlaviNiemitalo, we've been able to run a series of tests on a real VT420, so we have a good idea of how these ops are intended to work. Our implementation does a reasonably good job of matching that behavior, but we don't yet support paging, so we don't have the `DECCRA` ability to copy between pages, and we also don't have the concept of "unoccupied" cells, so we can't support that aspect of the streaming operations. It's also worth mentioning that the VT420 doesn't have colors, so we can't be sure exactly how they are meant to interpreted. However, based on the way the other attribute are handled, and what we know from the DEC STD 070 documentation, I think it's fair to assume that our handling of colors is also reasonable.
ghost
added
Resolution-Fix-Committed
|
🎉This issue was addressed in #14285, which has now been successfully released as Handy links: |
Description of the new feature/enhancement
This is a set of functions that include
DECFRA(fill rectangle),DECERA(erase rectangle),DECSERA(selective erase rectangle),DECCRA(copy rectangle),DECCARA(change attributes),DECRARA(reverse attributes), andDECSACE(this determines the affected area of the previous two operations - it can be a rectangle or a stream of characters).Some of these should be helpful in improving our console API support for passthrough mode (#1173 / #10001). Most useful would probably be
DECCRA,DECCARA, and possiblyDECFRA. We've also discussed usingDECCRAto support the layering concept in issue #10810, but that would require us supporting the paging functions as well (#13892).Proposed technical implementation details (optional)
I've got a working POC, and some of these are fairly straightforward, but some require low level access to the buffer. That means my current implementation will probably require some refactoring to work with the
ROWrewrite in PR #13626, so I'm happy to wait for that to merge first. AndDECSERAdepends on the new "protected" attribute added in PR #14046, so that definitely needs to merge first.I should also note that I'm not positive about all of the implementation details, so if possible I'd like to get someone with a real terminal to confirm the correct behavior before I submit a PR.
The text was updated successfully, but these errors were encountered: